Fix nil pointer panic when deployment replicas not set (#301)

Signed-off-by: zhujian <jiazhu@redhat.com>
open-cluster-management-io · Feb 27, 2025 · aede957 · aede957
1 parent 4329ebe
commit aede957
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 11 deletions.
diff --git a/pkg/addonmanager/controllers/agentdeploy/healthcheck_sync_test.go b/pkg/addonmanager/controllers/agentdeploy/healthcheck_sync_test.go
@@ -45,7 +45,8 @@ func (t *healthCheckTestAgent) Manifests(cluster *clusterv1.ManagedCluster,
 
 	return []runtime.Object{
 		NewFakeDeployment("test-deployment", "default"),
-		NewFakeZeroReplicasDeployment("test-zero-replicas-deployment", "default"),
+		NewFakeDeploymentZeroReplicas("test-deployment-zero-replicas", "default"),
+		NewFakeDeploymentReplicasNotSet("test-deployment-replicas-not-set", "default"),
 		NewFakeDaemonSet("test-daemonset", "default"),
 	}, nil
 }
@@ -57,12 +58,12 @@ func (t *healthCheckTestAgent) GetAgentAddonOptions() agent.AgentAddonOptions {
 	}
 }
 
-func NewFakeDeployment(namespace, name string) *appsv1.Deployment {
+func NewFakeDeployment(name, namespace string) *appsv1.Deployment {
 	var one int32 = 1
 	return &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      namespace,
-			Namespace: name,
+			Name:      name,
+			Namespace: namespace,
 		},
 		Spec: appsv1.DeploymentSpec{
 			Replicas: &one,
@@ -90,12 +91,12 @@ func NewFakeDeployment(namespace, name string) *appsv1.Deployment {
 	}
 }
 
-func NewFakeZeroReplicasDeployment(namespace, name string) *appsv1.Deployment {
+func NewFakeDeploymentZeroReplicas(name, namespace string) *appsv1.Deployment {
 	var zero int32 = 0
 	return &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      namespace,
-			Namespace: name,
+			Name:      name,
+			Namespace: namespace,
 		},
 		Spec: appsv1.DeploymentSpec{
 			Replicas: &zero,
@@ -123,11 +124,42 @@ func NewFakeZeroReplicasDeployment(namespace, name string) *appsv1.Deployment {
 	}
 }
 
-func NewFakeDaemonSet(namespace, name string) *appsv1.DaemonSet {
+func NewFakeDeploymentReplicasNotSet(name, namespace string) *appsv1.Deployment {
+	return &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		Spec: appsv1.DeploymentSpec{
+			Selector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"app": "test",
+				},
+			},
+			Template: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{
+						"addon": "test",
+					},
+				},
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Name:  "test",
+							Image: "test",
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+func NewFakeDaemonSet(name, namespace string) *appsv1.DaemonSet {
 	return &appsv1.DaemonSet{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      namespace,
-			Namespace: name,
+			Name:      name,
+			Namespace: namespace,
 		},
 		Spec: appsv1.DaemonSetSpec{
 			Selector: &metav1.LabelSelector{
@@ -736,6 +768,33 @@ func TestHealthCheckReconcile(t *testing.T) {
 										},
 									},
 								},
+								{
+									ResourceMeta: v1.ManifestResourceMeta{
+										Ordinal:   0,
+										Group:     "apps",
+										Version:   "",
+										Kind:      "",
+										Resource:  "deployments",
+										Name:      "test-deployment-replicas-not-set",
+										Namespace: "default",
+									},
+									StatusFeedbacks: v1.StatusFeedbackResult{
+										Values: []v1.FeedbackValue{
+											{
+												Name: "Replicas",
+												Value: v1.FieldValue{
+													Integer: boolPtr(1),
+												},
+											},
+											{
+												Name: "ReadyReplicas",
+												Value: v1.FieldValue{
+													Integer: boolPtr(1),
+												},
+											},
+										},
+									},
+								},
 							},
 						},
 						Conditions: []metav1.Condition{
@@ -973,6 +1032,33 @@ func TestHealthCheckReconcile(t *testing.T) {
 										},
 									},
 								},
+								{
+									ResourceMeta: v1.ManifestResourceMeta{
+										Ordinal:   0,
+										Group:     "apps",
+										Version:   "",
+										Kind:      "",
+										Resource:  "deployments",
+										Name:      "test-deployment-replicas-not-set",
+										Namespace: "default",
+									},
+									StatusFeedbacks: v1.StatusFeedbackResult{
+										Values: []v1.FeedbackValue{
+											{
+												Name: "Replicas",
+												Value: v1.FieldValue{
+													Integer: boolPtr(1),
+												},
+											},
+											{
+												Name: "ReadyReplicas",
+												Value: v1.FieldValue{
+													Integer: boolPtr(1),
+												},
+											},
+										},
+									},
+								},
 								{
 									ResourceMeta: v1.ManifestResourceMeta{
 										Ordinal:   0,

diff --git a/pkg/utils/probe_helper.go b/pkg/utils/probe_helper.go
@@ -187,6 +187,12 @@ func FilterWorkloads(objects []runtime.Object) []WorkloadMetadata {
 	for _, obj := range objects {
 		deployment, err := ConvertToDeployment(obj)
 		if err == nil {
+			// deployment replicas defaults to 1
+			// https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#replicas
+			var deploymentReplicas int32 = 1
+			if deployment.Spec.Replicas != nil {
+				deploymentReplicas = *deployment.Spec.Replicas
+			}
 			workloads = append(workloads, WorkloadMetadata{
 				GroupResource: schema.GroupResource{
 					Group:    appsv1.GroupName,
@@ -197,7 +203,7 @@ func FilterWorkloads(objects []runtime.Object) []WorkloadMetadata {
 					Name:      deployment.Name,
 				},
 				DeploymentSpec: &DeploymentSpec{
-					Replicas: *deployment.Spec.Replicas,
+					Replicas: deploymentReplicas,
 				},
 			})
 		}