When the operator restarts and adds defaultimagepullsecrets, the pod …

[jiuker]

When the operator restarts and adds defaultimagepullsecrets, the pod does not update imagepullsecrets in a timely manner. If MQ is switched or down, it may restart and fail to pull down the image
steps:
1.start operator with pull serect names
2.start mq instance
3.restart operator with new pull serect names

we found that mq instance won't restart to refresh pull serect key.
But if step1 with no serect names,step3 will make mq instance restart。That's is a bug, I think.

case by

if rabbitmqCluster.Spec.ImagePullSecrets == nil {
 		// split the comma separated list of default image pull secrets from
 		// the 'DEFAULT_IMAGE_PULL_SECRETS' env var, but ignore empty strings.
 		for _, reference := range strings.Split(r.DefaultImagePullSecrets, ",") {
 			if len(reference) > 0 {
 				rabbitmqCluster.Spec.ImagePullSecrets = append(rabbitmqCluster.Spec.ImagePullSecrets, corev1.LocalObjectReference{Name: reference})
 			}
 		}
 		if requeue, err := r.updateRabbitmqCluster(ctx, rabbitmqCluster, "image pull secrets"); err != nil {
 			return requeue, err
 		}
}

[jiuker]

@coro review?

[Zerpet]

I'm afraid we can't accept this contribution in its current state, due to the following reasons:

1. Your commit messages do not adhere to our contribution guidelines regarding descriptive commit messages
2. There are no new tests to ensure the correct behaviour of this new code

I'm not sure I understand why this would be a bug.

In scenario 1, a given RabbitmqCluster object will have pull secrets, as inherited from Operator's default pull secrets. After updating the Operator and a restart, the Operator should respect the previosly set pull secrets. This behaviour is common for other fields in the Spec.

In scenario 2, a given RabbitmqCluster object will have no pull secrets, as those were not set in the Spec, neither in the Operator default pull secrets. There could be an argument in favour of leaving pull secrets unset, as those were unset in a previous loop. However, it's not trivial to distinguish between set to nil and not set at all. That's the reason why it updates after an Operator restart. I believe this is a desirable behaviour.