Uploaded image for project: 'OpenShift Bugs'
  1. OpenShift Bugs
  2. OCPBUGS-22209

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

XMLWordPrintable

    • Important
    • No
    • 1
    • Metal Platform 244, Metal Platform 245
    • 2
    • False
    • Hide

      None

      Show
      None
    • 8/22: PR under review; green; KNIECO-8033

      This is a clone of issue OCPBUGS-16919. The following is the description of the original issue:

      Description of problem:

      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

      Version-Release number of selected component (if applicable):

      Openshift-Baremetal-Operator

      Steps to Reproduce:

      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. 

      Actual results:

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

      Expected results:

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

      Additional info:

       

              rpittau@redhat.com Riccardo Pittau
              openshift-crt-jira-prow OpenShift Prow Bot
              Jad Haj Yahya Jad Haj Yahya
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Created:
                Updated:
                Resolved: