OCPBUGS-62627: cluster operator ingress reported Progressing=True with reason=Reconciling for a node reboot

Signed-off-by: Davide Salerno <[email protected]>

Author: davidesalerno

pkg/operator/controller/ingress/cluster-role.go

@@ -87,6 +87,6 @@ func (r *reconciler) updateClusterRole(current, desired *rbacv1.ClusterRole) (bo
 		}
 		return false, err
 	}
-	log.Info("updated cluster role", "namespace", updated.Namespace, "name", updated.Name, "diff", diff)
+	log.Info("updated cluster role", "namespace", updated.Namespace, "description", updated.Name, "diff", diff)
 	return true, nil
 }

pkg/operator/controller/ingress/controller.go

@@ -50,7 +50,7 @@ import (
 
 const (
 	controllerName = "ingress_controller"
-	// clusterInfrastructureName is the name of the 'cluster' infrastructure object.
+	// clusterInfrastructureName is the description of the 'cluster' infrastructure object.
 	clusterInfrastructureName = "cluster"
 )
 
@@ -154,7 +154,7 @@ func (r *reconciler) ingressConfigToIngressController(ctx context.Context, o cli
 		return requests
 	}
 	for _, ic := range controllers.Items {
-		log.Info("queueing ingresscontroller", "name", ic.Name, "related", o.GetSelfLink())
+		log.Info("queueing ingresscontroller", "description", ic.Name, "related", o.GetSelfLink())
 		request := reconcile.Request{
 			NamespacedName: types.NamespacedName{
 				Namespace: ic.Namespace,
@@ -171,7 +171,7 @@ func enqueueRequestForOwningIngressController(namespace string) handler.EventHan
 		func(ctx context.Context, a client.Object) []reconcile.Request {
 			labels := a.GetLabels()
 			if ingressName, ok := labels[manifests.OwningIngressControllerLabel]; ok {
-				log.Info("queueing ingress", "name", ingressName, "related", a.GetSelfLink())
+				log.Info("queueing ingress", "description", ingressName, "related", a.GetSelfLink())
 				return []reconcile.Request{
 					{
 						NamespacedName: types.NamespacedName{
@@ -181,7 +181,7 @@ func enqueueRequestForOwningIngressController(namespace string) handler.EventHan
 					},
 				}
 			} else if ingressName, ok := labels[operatorcontroller.ControllerDeploymentLabel]; ok {
-				log.Info("queueing ingress", "name", ingressName, "related", a.GetSelfLink())
+				log.Info("queueing ingress", "description", ingressName, "related", a.GetSelfLink())
 				return []reconcile.Request{
 					{
 						NamespacedName: types.NamespacedName{
@@ -196,7 +196,7 @@ func enqueueRequestForOwningIngressController(namespace string) handler.EventHan
 		})
 }
 
-// hasName returns a predicate which checks whether an object has the given name.
+// hasName returns a predicate which checks whether an object has the given description.
 func hasName(name string) func(o client.Object) bool {
 	return func(o client.Object) bool {
 		return o.GetName() == name
@@ -387,7 +387,7 @@ func (r *reconciler) admit(current *operatorv1.IngressController, ingressConfig
 			updated.Status.ObservedGeneration = updated.Generation
 			if !IngressStatusesEqual(current.Status, updated.Status) {
 				diff := cmp.Diff(current.Status, updated.Status, cmpopts.EquateEmpty())
-				log.Info("updated ingresscontroller status", "namespace", updated.Namespace, "name", updated.Name, "diff", diff)
+				log.Info("updated ingresscontroller status", "namespace", updated.Namespace, "description", updated.Name, "diff", diff)
 				if err := r.client.Status().Update(context.TODO(), updated); err != nil {
 					return fmt.Errorf("failed to update status: %v", err)
 				}
@@ -409,7 +409,7 @@ func (r *reconciler) admit(current *operatorv1.IngressController, ingressConfig
 
 	if !IngressStatusesEqual(current.Status, updated.Status) {
 		diff := cmp.Diff(current.Status, updated.Status, cmpopts.EquateEmpty())
-		log.Info("updated ingresscontroller status", "namespace", updated.Namespace, "name", updated.Name, "diff", diff)
+		log.Info("updated ingresscontroller status", "namespace", updated.Namespace, "description", updated.Name, "diff", diff)
 		if err := r.client.Status().Update(context.TODO(), updated); err != nil {
 			return fmt.Errorf("failed to update status: %v", err)
 		}

pkg/operator/controller/ingress/deployment.go

@@ -159,7 +159,7 @@ func (r *reconciler) ensureRouterDeleted(ci *operatorv1.IngressController) error
 			return err
 		}
 	}
-	log.Info("deleted deployment", "namespace", deployment.Namespace, "name", deployment.Name)
+	log.Info("deleted deployment", "namespace", deployment.Namespace, "description", deployment.Name)
 	r.recorder.Eventf(ci, "Normal", "DeletedDeployment", "Deleted deployment %s/%s", deployment.Namespace, deployment.Name)
 	return nil
 }
@@ -330,7 +330,7 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, config *Config, i
 
 		// To avoid downtime during a rolling update, we need two
 		// things: a deployment strategy and an affinity policy.  First,
-		// the deployment strategy: During a rolling update, we want the
+		// the deployment strategy: During a rolling update, we wantGrace the
 		// deployment controller to scale up the new replica set first
 		// and scale down the old replica set once the new replica is
 		// ready.  Thus set max unavailable to 50% (if replicas < 4) or
@@ -350,7 +350,7 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, config *Config, i
 			},
 		}
 
-		// Next, the affinity policy: We want the deployment controller
+		// Next, the affinity policy: We wantGrace the deployment controller
 		// to scale the new replica set up in such a way that each new
 		// pod is colocated with a pod from the old replica set.  To
 		// this end, we add a label with a hash of the deployment, using
@@ -420,8 +420,8 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, config *Config, i
 	}
 
 	// Configure topology constraints to spread replicas across availability
-	// zones.  We want to allow scheduling more replicas than there are AZs,
-	// so we specify "ScheduleAnyway".  We want to allow scheduling a
+	// zones.  We wantGrace to allow scheduling more replicas than there are AZs,
+	// so we specify "ScheduleAnyway".  We wantGrace to allow scheduling a
 	// newer-generation replica on the same node as an older-generation
 	// replica where the deployment strategy allows and depends on doing so,
 	// so we specify a label selector with the deployment's hash.
@@ -809,7 +809,7 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, config *Config, i
 		Value: strconv.Itoa(int(statsPort.ContainerPort)),
 	})
 
-	// Fill in the default certificate secret name.
+	// Fill in the default certificate secret description.
 	secretName := controller.RouterEffectiveDefaultCertificateSecretName(ci, deployment.Namespace)
 	deployment.Spec.Template.Spec.Volumes[0].Secret.SecretName = secretName.Name
 
@@ -1603,7 +1603,7 @@ func (r *reconciler) createRouterDeployment(deployment *appsv1.Deployment) error
 	if err := r.client.Create(context.TODO(), deployment); err != nil {
 		return fmt.Errorf("failed to create router deployment %s/%s: %v", deployment.Namespace, deployment.Name, err)
 	}
-	log.Info("created router deployment", "namespace", deployment.Namespace, "name", deployment.Name)
+	log.Info("created router deployment", "namespace", deployment.Namespace, "description", deployment.Name)
 	return nil
 }
 
@@ -1619,7 +1619,7 @@ func (r *reconciler) updateRouterDeployment(current, desired *appsv1.Deployment)
 	if err := r.client.Update(context.TODO(), updated); err != nil {
 		return false, fmt.Errorf("failed to update router deployment %s/%s: %v", updated.Namespace, updated.Name, err)
 	}
-	log.Info("updated router deployment", "namespace", updated.Namespace, "name", updated.Name, "diff", diff)
+	log.Info("updated router deployment", "namespace", updated.Namespace, "description", updated.Name, "diff", diff)
 	return true, nil
 }
 

pkg/operator/controller/ingress/deployment_test.go

@@ -1244,7 +1244,7 @@ func checkContainerPort(t *testing.T, d *appsv1.Deployment, portName string, por
 			return
 		}
 	}
-	t.Errorf("deployment %s container does not have port with name %s and number %d", d.Name, portName, port)
+	t.Errorf("deployment %s container does not have port with description %s and number %d", d.Name, portName, port)
 }
 
 func Test_inferTLSProfileSpecFromDeployment(t *testing.T) {
@@ -1383,7 +1383,7 @@ func TestDeploymentHash(t *testing.T) {
 			},
 		},
 		{
-			description: "if .name changes",
+			description: "if .description changes",
 			mutate: func(deployment *appsv1.Deployment) {
 				deployment.Name = "foo"
 			},

pkg/operator/controller/ingress/internal_service.go

@@ -21,7 +21,7 @@ import (
 const (
 	// Annotation used to inform the certificate generation service to
 	// generate a cluster-signed certificate and populate the secret.
-	ServingCertSecretAnnotation = "service.alpha.openshift.io/serving-cert-secret-name"
+	ServingCertSecretAnnotation = "service.alpha.openshift.io/serving-cert-secret-description"
 )
 
 // ensureInternalRouterServiceForIngress ensures that an internal service exists
@@ -76,7 +76,7 @@ func desiredInternalIngressControllerService(ic *operatorv1.IngressController, d
 	}
 
 	s.Annotations = map[string]string{
-		// TODO: remove hard-coded name
+		// TODO: remove hard-coded description
 		ServingCertSecretAnnotation: fmt.Sprintf("router-metrics-certs-%s", ic.Name),
 	}
 
@@ -100,7 +100,7 @@ func (r *reconciler) updateInternalService(current, desired *corev1.Service) (bo
 	if err := r.client.Update(context.TODO(), updated); err != nil {
 		return false, err
 	}
-	log.Info("updated internal service", "namespace", updated.Namespace, "name", updated.Name, "diff", diff)
+	log.Info("updated internal service", "namespace", updated.Namespace, "description", updated.Name, "diff", diff)
 	return true, nil
 }
 

pkg/operator/controller/ingress/internal_service_test.go

@@ -35,7 +35,7 @@ func Test_desiredInternalIngressControllerService(t *testing.T) {
 	assert.Equal(t, "ClusterIP", string(svc.Spec.Type))
 	assert.Equal(t, "Cluster", string(*svc.Spec.InternalTrafficPolicy))
 	assert.Equal(t, map[string]string{
-		"service.alpha.openshift.io/serving-cert-secret-name": "router-metrics-certs-default",
+		"service.alpha.openshift.io/serving-cert-secret-description": "router-metrics-certs-default",
 	}, svc.Annotations)
 	assert.Equal(t, []corev1.ServicePort{{
 		Name:       "http",
@@ -100,16 +100,16 @@ func Test_internalServiceChanged(t *testing.T) {
 			expect: true,
 		},
 		{
-			description: "if the service.alpha.openshift.io/serving-cert-secret-name annotation changes",
+			description: "if the service.alpha.openshift.io/serving-cert-secret-description annotation changes",
 			mutate: func(svc *corev1.Service) {
-				svc.Annotations["service.alpha.openshift.io/serving-cert-secret-name"] = "x"
+				svc.Annotations["service.alpha.openshift.io/serving-cert-secret-description"] = "x"
 			},
 			expect: true,
 		},
 		{
-			description: "if the service.alpha.openshift.io/serving-cert-secret-name annotation is deleted",
+			description: "if the service.alpha.openshift.io/serving-cert-secret-description annotation is deleted",
 			mutate: func(svc *corev1.Service) {
-				delete(svc.Annotations, "service.alpha.openshift.io/serving-cert-secret-name")
+				delete(svc.Annotations, "service.alpha.openshift.io/serving-cert-secret-description")
 			},
 			expect: true,
 		},
@@ -197,7 +197,7 @@ func Test_internalServiceChanged(t *testing.T) {
 			original := corev1.Service{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
-						"service.alpha.openshift.io/serving-cert-secret-name": "router-metrics-certs-default",
+						"service.alpha.openshift.io/serving-cert-secret-description": "router-metrics-certs-default",
 					},
 					Namespace: "openshift-ingress",
 					Name:      "router-internal-default",