[release-4.13] Machine API pull-secret is not updated if the global pull-secret changes as a Day-2 operation.

The pull-secret in the openshift-machine-api namespace is getting copied from the global pull-secret (openshift-config/pull-secret) during installation of the cluster. Although if the global pull-secret change as it is described in our documentation [0] the pull-secret will not get updated. This has as a consiquence a BMH to stuck in provisioning state as the first-boot ignition that gets from the BareMetal Operator contains the old pull-secret and the ironic-agent service is not able to pull the image and start.

More specifically:

The function applySecret [1] logic is that it wont update the secret but only copy it if it does not exist when the provided callback function `shouldUpdateData shouldUpdateDataFn` returns false.

The applySecret function will be called from createRegistryPullSecret [2] and it will get as input the parameter "doNotUpdateData" which always returns false

~~~
return applySecret(info.Client.CoreV1(), info.EventRecorder, secret, doNotUpdateData)
~~~
Here it is called in the return of createRegistryPullSecret function.

And the doNotUpdateData is set to false
~~~
func doNotUpdateData(existing *corev1.Secret) (bool, error) {
    return false, nil
}
~~~

So the applySecret will never update the secret. It will only replace it if it does not exist.

~~~
func applySecret(client coreclientv1.SecretsGetter, recorder events.Recorder, requiredInput *corev1.Secret, shouldUpdateData shouldUpdateDataFn) error {
    needsApply := false                                                   <---- Here sets the needApply to false
    existing, err := client.Secrets(requiredInput.Namespace).Get(context.TODO(), requiredInput.Name, metav1.GetOptions{})
    if apierrors.IsNotFound(err) {
        err = nil
        needsApply = true
    } else if err != nil {
        return err
    } else {
        // Allow the caller to decide whether update.
        needsApply, err = shouldUpdateData(existing)  <---- Here this paremeter (shouldUpdateData) is referring to the doNotUpdateData that is false
        if err != nil {
            return err
        }
    }    if needsApply {
        _, _, err = resourceapply.ApplySecret(context.TODO(), client, recorder, requiredInput) <--- so this will not run.
    }    return err
}
~~~

So i wonder if this is meant to be like this for some reason.

I think we need either to update the documentation to instruct the user to delete the pull-secret in the openshift-machine-api when changing the global pull-secret OR to enable the applySecret to be able to also apply and not only to create the pull-secret if the global pull-secret is changed.

From my perspective i think the second option is better.

Let me know if im missing something in the above explanation.

[0] https://docs.openshift.com/container-platform/4.12/openshift_images/managing_images/using-image-pull-secrets.html#images-update-global-pull-secret_using-image-pull-secrets
[1] https://github.com/openshift/cluster-baremetal-operator/blob/f15e1c53cd5740d1ef7f82c8364e5d8e97c9064a/provisioning/baremetal_secrets.go#L48
[2] https://github.com/openshift/cluster-baremetal-operator/blob/f15e1c53cd5740d1ef7f82c8364e5d8e97c9064a/provisioning/baremetal_secrets.go#L120C10-L120C10

Openshift-Baremetal-Operator

1) On a Bare Metal IPI cluster update the global pull-secret using the documentation page [0].
2) Check the secret pull-secret inside the openshift-machine-api namespace and it would be the old one.
3) Delete the secret pull-secret insid the openshift-machine-api namespace 
4) Notice that it will be recreated with the updated pull-secret. 

The machine-api secret pull-secret is not updated after global pull-secret change

The machine-api secret pull-secret is updated after global pull-secret change