Fix nil pointer dereference on job details page (kubernetes#2302)

* Fix `nil pointer dereference` on job details page

* Make tests compile

* Fix test

* Fix test

* Fix tests

* Fix tests

src/app/backend/resource/common/podinfo.go

@@ -22,7 +22,7 @@ type PodInfo struct {
 	Current int32 `json:"current"`
 
 	// Number of pods that are desired.
-	Desired int32 `json:"desired"`
+	Desired *int32 `json:"desired,omitempty"`
 
 	// Number of pods that are currently running.
 	Running int32 `json:"running"`
@@ -41,7 +41,7 @@ type PodInfo struct {
 }
 
 // GetPodInfo returns aggregate information about a group of pods.
-func GetPodInfo(current int32, desired int32, pods []api.Pod) PodInfo {
+func GetPodInfo(current int32, desired *int32, pods []api.Pod) PodInfo {
 	result := PodInfo{
 		Current:  current,
 		Desired:  desired,

src/app/backend/resource/common/podinfo_test.go

@@ -21,15 +21,20 @@ import (
 	api "k8s.io/client-go/pkg/api/v1"
 )
 
+func getReplicasPointer(replicas int32) *int32 {
+	return &replicas
+}
+
 func TestGetPodInfo(t *testing.T) {
 	cases := []struct {
-		current, desired int32
-		pods             []api.Pod
-		expected         PodInfo
+		current  int32
+		desired  *int32
+		pods     []api.Pod
+		expected PodInfo
 	}{
 		{
 			5,
-			4,
+			getReplicasPointer(4),
 			[]api.Pod{
 				{
 					Status: api.PodStatus{
@@ -39,7 +44,7 @@ func TestGetPodInfo(t *testing.T) {
 			},
 			PodInfo{
 				Current:  5,
-				Desired:  4,
+				Desired:  getReplicasPointer(4),
 				Running:  1,
 				Pending:  0,
 				Failed:   0,

src/app/backend/resource/controller/controller.go

@@ -106,11 +106,7 @@ type JobController batch.Job
 // Get is an implementation of Get method from ResourceController interface.
 func (self JobController) Get(allPods []v1.Pod, allEvents []v1.Event) ResourceOwner {
 	matchingPods := common.FilterPodsForJob(batch.Job(self), allPods)
-	var completions int32
-	if self.Spec.Completions != nil {
-		completions = *self.Spec.Completions
-	}
-	podInfo := common.GetPodInfo(self.Status.Active, completions, matchingPods)
+	podInfo := common.GetPodInfo(self.Status.Active, self.Spec.Completions, matchingPods)
 	podInfo.Warnings = event.GetPodsEventWarnings(allEvents, matchingPods)
 
 	return ResourceOwner{
@@ -142,7 +138,7 @@ type ReplicaSetController extensions.ReplicaSet
 // Get is an implementation of Get method from ResourceController interface.
 func (self ReplicaSetController) Get(allPods []v1.Pod, allEvents []v1.Event) ResourceOwner {
 	matchingPods := common.FilterPodsByControllerRef(&self, allPods)
-	podInfo := common.GetPodInfo(self.Status.Replicas, *self.Spec.Replicas, matchingPods)
+	podInfo := common.GetPodInfo(self.Status.Replicas, self.Spec.Replicas, matchingPods)
 	podInfo.Warnings = event.GetPodsEventWarnings(allEvents, matchingPods)
 
 	return ResourceOwner{
@@ -175,7 +171,7 @@ type ReplicationControllerController v1.ReplicationController
 func (self ReplicationControllerController) Get(allPods []v1.Pod,
 	allEvents []v1.Event) ResourceOwner {
 	matchingPods := common.FilterPodsByControllerRef(&self, allPods)
-	podInfo := common.GetPodInfo(self.Status.Replicas, *self.Spec.Replicas, matchingPods)
+	podInfo := common.GetPodInfo(self.Status.Replicas, self.Spec.Replicas, matchingPods)
 	podInfo.Warnings = event.GetPodsEventWarnings(allEvents, matchingPods)
 
 	return ResourceOwner{
@@ -208,7 +204,7 @@ type DaemonSetController extensions.DaemonSet
 func (self DaemonSetController) Get(allPods []v1.Pod, allEvents []v1.Event) ResourceOwner {
 	matchingPods := common.FilterPodsByControllerRef(&self, allPods)
 	podInfo := common.GetPodInfo(self.Status.CurrentNumberScheduled,
-		self.Status.DesiredNumberScheduled, matchingPods)
+		&self.Status.DesiredNumberScheduled, matchingPods)
 	podInfo.Warnings = event.GetPodsEventWarnings(allEvents, matchingPods)
 
 	return ResourceOwner{
@@ -240,7 +236,7 @@ type StatefulSetController apps.StatefulSet
 // Get is an implementation of Get method from ResourceController interface.
 func (self StatefulSetController) Get(allPods []v1.Pod, allEvents []v1.Event) ResourceOwner {
 	matchingPods := common.FilterPodsByControllerRef(&self, allPods)
-	podInfo := common.GetPodInfo(self.Status.Replicas, *self.Spec.Replicas, matchingPods)
+	podInfo := common.GetPodInfo(self.Status.Replicas, self.Spec.Replicas, matchingPods)
 	podInfo.Warnings = event.GetPodsEventWarnings(allEvents, matchingPods)
 
 	return ResourceOwner{

src/app/backend/resource/daemonset/list.go

@@ -112,7 +112,7 @@ func toDaemonSetList(daemonSets []extensions.DaemonSet, pods []v1.Pod, events []
 	for _, daemonSet := range daemonSets {
 		matchingPods := common.FilterPodsByControllerRef(&daemonSet, pods)
 		podInfo := common.GetPodInfo(daemonSet.Status.CurrentNumberScheduled,
-			daemonSet.Status.DesiredNumberScheduled, matchingPods)
+			&daemonSet.Status.DesiredNumberScheduled, matchingPods)
 		podInfo.Warnings = event.GetPodsEventWarnings(events, matchingPods)
 
 		daemonSetList.DaemonSets = append(daemonSetList.DaemonSets, DaemonSet{

src/app/backend/resource/daemonset/list_test.go

@@ -30,6 +30,7 @@ import (
 func TestToDaemonSetList(t *testing.T) {
 	events := []v1.Event{}
 	controller := true
+	var desired int32 = 1
 	validPodMeta := metaV1.ObjectMeta{
 		Namespace: "namespace-1",
 		OwnerReferences: []metaV1.OwnerReference{
@@ -60,9 +61,13 @@ func TestToDaemonSetList(t *testing.T) {
 		nodes      []v1.Node
 		expected   *DaemonSetList
 	}{
-		{nil, nil, nil, nil, &DaemonSetList{
-			DaemonSets:        []DaemonSet{},
-			CumulativeMetrics: make([]metricapi.Metric, 0)},
+		{nil,
+			nil,
+			nil,
+			nil,
+			&DaemonSetList{
+				DaemonSets:        []DaemonSet{},
+				CumulativeMetrics: make([]metricapi.Metric, 0)},
 		}, {
 			[]extensions.DaemonSet{
 				{
@@ -79,6 +84,9 @@ func TestToDaemonSetList(t *testing.T) {
 							Spec: v1.PodSpec{Containers: []v1.Container{{Image: "my-container-image-1"}}},
 						},
 					},
+					Status: extensions.DaemonSetStatus{
+						DesiredNumberScheduled: desired,
+					},
 				},
 				{
 					ObjectMeta: metaV1.ObjectMeta{
@@ -93,6 +101,9 @@ func TestToDaemonSetList(t *testing.T) {
 							Spec: v1.PodSpec{Containers: []v1.Container{{Image: "my-container-image-2"}}},
 						},
 					},
+					Status: extensions.DaemonSetStatus{
+						DesiredNumberScheduled: desired,
+					},
 				},
 			},
 			[]v1.Service{
@@ -178,6 +189,7 @@ func TestToDaemonSetList(t *testing.T) {
 						TypeMeta:        api.TypeMeta{Kind: api.ResourceKindDaemonSet},
 						ContainerImages: []string{"my-container-image-1"},
 						Pods: common.PodInfo{
+							Desired:   &desired,
 							Failed:    2,
 							Pending:   1,
 							Running:   1,
@@ -192,6 +204,7 @@ func TestToDaemonSetList(t *testing.T) {
 						TypeMeta:        api.TypeMeta{Kind: api.ResourceKindDaemonSet},
 						ContainerImages: []string{"my-container-image-2"},
 						Pods: common.PodInfo{
+							Desired:  &desired,
 							Warnings: []common.Event{},
 						},
 					},

src/app/backend/resource/daemonset/pods.go

@@ -79,6 +79,6 @@ func getDaemonSetPodInfo(client k8sClient.Interface, daemonSet *extensions.Daemo
 	}
 
 	podInfo := common.GetPodInfo(daemonSet.Status.CurrentNumberScheduled,
-		daemonSet.Status.DesiredNumberScheduled, pods)
+		&daemonSet.Status.DesiredNumberScheduled, pods)
 	return &podInfo, nil
 }

src/app/backend/resource/deployment/detail.go

@@ -107,7 +107,6 @@ func GetDeploymentDetail(client client.Interface, metricClient metricapi.MetricC
 		return nil, err
 	}
 
-	// TODO fix not to use kubernetes utils
 	selector, err := metaV1.LabelSelectorAsSelector(deployment.Spec.Selector)
 	if err != nil {
 		return nil, err
@@ -172,7 +171,7 @@ func GetDeploymentDetail(client client.Interface, metricClient metricapi.MetricC
 	var newReplicaSet replicaset.ReplicaSet
 	if newRs != nil {
 		matchingPods := common.FilterPodsByControllerRef(newRs, rawPods.Items)
-		newRsPodInfo := common.GetPodInfo(newRs.Status.Replicas, *newRs.Spec.Replicas, matchingPods)
+		newRsPodInfo := common.GetPodInfo(newRs.Status.Replicas, newRs.Spec.Replicas, matchingPods)
 		events, err := event.GetPodsEvents(client, namespace, matchingPods)
 		nonCriticalErrors, criticalError = errors.AppendError(err, nonCriticalErrors)
 		if criticalError != nil {

src/app/backend/resource/deployment/detail_test.go

@@ -32,9 +32,8 @@ import (
 	extensions "k8s.io/client-go/pkg/apis/extensions/v1beta1"
 )
 
-func createDeployment(name, namespace, podTemplateName string, podLabel,
+func createDeployment(name, namespace, podTemplateName string, replicas int32, podLabel,
 	labelSelector map[string]string) *extensions.Deployment {
-	replicas := int32(4)
 	maxSurge := intstr.FromInt(1)
 	maxUnavailable := intstr.FromString("25%")
 
@@ -43,8 +42,9 @@ func createDeployment(name, namespace, podTemplateName string, podLabel,
 			Name: name, Namespace: namespace, Labels: labelSelector,
 		},
 		Spec: extensions.DeploymentSpec{
-			Selector: &metaV1.LabelSelector{MatchLabels: labelSelector},
-			Replicas: &replicas, MinReadySeconds: 5,
+			Selector:        &metaV1.LabelSelector{MatchLabels: labelSelector},
+			Replicas:        &replicas,
+			MinReadySeconds: 5,
 			Strategy: extensions.DeploymentStrategy{
 				Type: extensions.RollingUpdateDeploymentStrategyType,
 				RollingUpdate: &extensions.RollingUpdateDeployment{
@@ -56,14 +56,13 @@ func createDeployment(name, namespace, podTemplateName string, podLabel,
 				ObjectMeta: metaV1.ObjectMeta{Name: podTemplateName, Labels: podLabel}},
 		},
 		Status: extensions.DeploymentStatus{
-			Replicas: 4, UpdatedReplicas: 2, AvailableReplicas: 3, UnavailableReplicas: 1,
+			Replicas: replicas, UpdatedReplicas: 2, AvailableReplicas: 3, UnavailableReplicas: 1,
 		},
 	}
 }
 
-func createReplicaSet(name, namespace string, labelSelector map[string]string,
+func createReplicaSet(name, namespace string, replicas int32, labelSelector map[string]string,
 	podTemplateSpec v1.PodTemplateSpec) extensions.ReplicaSet {
-	replicas := int32(0)
 	return extensions.ReplicaSet{
 		ObjectMeta: metaV1.ObjectMeta{
 			Name: name, Namespace: namespace, Labels: labelSelector,
@@ -78,18 +77,20 @@ func createReplicaSet(name, namespace string, labelSelector map[string]string,
 func TestGetDeploymentDetail(t *testing.T) {
 	podList := &v1.PodList{}
 	eventList := &v1.EventList{}
+	var replicas int32 = 4
 
-	deployment := createDeployment("dp-1", "ns-1", "pod-1", map[string]string{"track": "beta"},
-		map[string]string{"foo": "bar"})
+	deployment := createDeployment("dp-1", "ns-1", "pod-1", replicas,
+		map[string]string{"track": "beta"}, map[string]string{"foo": "bar"})
 
 	podTemplateSpec := GetNewReplicaSetTemplate(deployment)
 
-	newReplicaSet := createReplicaSet("rs-1", "ns-1", map[string]string{"foo": "bar"}, podTemplateSpec)
+	newReplicaSet := createReplicaSet("rs-1", "ns-1", replicas, map[string]string{"foo": "bar"},
+		podTemplateSpec)
 
 	replicaSetList := &extensions.ReplicaSetList{
 		Items: []extensions.ReplicaSet{
 			newReplicaSet,
-			createReplicaSet("rs-2", "ns-1", map[string]string{"foo": "bar"},
+			createReplicaSet("rs-2", "ns-1", replicas, map[string]string{"foo": "bar"},
 				podTemplateSpec),
 		},
 	}
@@ -140,7 +141,10 @@ func TestGetDeploymentDetail(t *testing.T) {
 				NewReplicaSet: replicaset.ReplicaSet{
 					ObjectMeta: api.NewObjectMeta(newReplicaSet.ObjectMeta),
 					TypeMeta:   api.NewTypeMeta(api.ResourceKindReplicaSet),
-					Pods:       common.PodInfo{Warnings: []common.Event{}},
+					Pods: common.PodInfo{
+						Warnings: []common.Event{},
+						Desired:  &replicas,
+					},
 				},
 				EventList: common.EventList{
 					Events: []common.Event{},

src/app/backend/resource/deployment/list.go

@@ -124,7 +124,7 @@ func toDeploymentList(deployments []extensions.Deployment, pods []v1.Pod, events
 
 	for _, deployment := range deployments {
 		matchingPods := common.FilterDeploymentPodsByOwnerReference(deployment, rs, pods)
-		podInfo := common.GetPodInfo(deployment.Status.Replicas, *deployment.Spec.Replicas, matchingPods)
+		podInfo := common.GetPodInfo(deployment.Status.Replicas, deployment.Spec.Replicas, matchingPods)
 		podInfo.Warnings = event.GetPodsEventWarnings(events, matchingPods)
 
 		deploymentList.Deployments = append(deploymentList.Deployments,

src/app/backend/resource/deployment/list_test.go

@@ -114,7 +114,7 @@ func TestGetDeploymentListFromChannels(t *testing.T) {
 					TypeMeta: api.TypeMeta{Kind: api.ResourceKindDeployment},
 					Pods: common.PodInfo{
 						Current:  7,
-						Desired:  21,
+						Desired:  getReplicasPointer(21),
 						Failed:   0,
 						Warnings: []common.Event{},
 					},

src/app/backend/resource/job/detail_test.go

@@ -28,8 +28,7 @@ import (
 	batch "k8s.io/client-go/pkg/apis/batch/v1"
 )
 
-func createJob(name, namespace string, labelSelector map[string]string) *batch.Job {
-	var jobCompletions int32
+func createJob(name, namespace string, jobCompletions int32, labelSelector map[string]string) *batch.Job {
 	var parallelism int32
 
 	return &batch.Job{
@@ -57,12 +56,15 @@ func TestGetJobDetail(t *testing.T) {
 		{
 			"ns-1", "job-1",
 			[]string{"get", "get", "list", "list", "list", "list"},
-			createJob("job-1", "ns-1", map[string]string{"app": "test"}),
+			createJob("job-1", "ns-1", jobCompletions, map[string]string{"app": "test"}),
 			&JobDetail{
 				ObjectMeta: api.ObjectMeta{Name: "job-1", Namespace: "ns-1",
 					Labels: map[string]string{"app": "test"}},
 				TypeMeta: api.TypeMeta{Kind: api.ResourceKindJob},
-				PodInfo:  common.PodInfo{Warnings: []common.Event{}},
+				PodInfo: common.PodInfo{
+					Warnings: []common.Event{},
+					Desired:  &jobCompletions,
+				},
 				PodList: pod.PodList{
 					Pods:              []pod.Pod{},
 					CumulativeMetrics: make([]metricapi.Metric, 0),

src/app/backend/resource/job/events_test.go

@@ -28,6 +28,7 @@ import (
 )
 
 func TestGetJobEvents(t *testing.T) {
+	var jobCompletions int32
 	cases := []struct {
 		namespace, name string
 		eventList       *v1.EventList
@@ -46,7 +47,7 @@ func TestGetJobEvents(t *testing.T) {
 			&v1.PodList{Items: []v1.Pod{{ObjectMeta: metaV1.ObjectMeta{
 				Name: "pod-1", Namespace: "ns-1",
 			}}}},
-			createJob("job-1", "ns-1", map[string]string{"app": "test"}),
+			createJob("job-1", "ns-1", jobCompletions, map[string]string{"app": "test"}),
 			[]string{"list"},
 			&common.EventList{
 				ListMeta: api.ListMeta{TotalItems: 1},

src/app/backend/resource/job/list.go

@@ -116,12 +116,8 @@ func toJobList(jobs []batch.Job, pods []v1.Pod, events []v1.Event, nonCriticalEr
 	jobList.ListMeta = api.ListMeta{TotalItems: filteredTotal}
 
 	for _, job := range jobs {
-		var completions int32
 		matchingPods := common.FilterPodsForJob(job, pods)
-		if job.Spec.Completions != nil {
-			completions = *job.Spec.Completions
-		}
-		podInfo := common.GetPodInfo(job.Status.Active, completions, matchingPods)
+		podInfo := common.GetPodInfo(job.Status.Active, job.Spec.Completions, matchingPods)
 		podInfo.Warnings = event.GetPodsEventWarnings(events, matchingPods)
 		jobList.Jobs = append(jobList.Jobs, toJob(&job, &podInfo))
 	}

src/app/backend/resource/job/list_test.go

@@ -30,7 +30,7 @@ import (
 )
 
 func TestGetJobListFromChannels(t *testing.T) {
-	var jobCompletions int32 = 21
+	var completions int32 = 21
 	controller := true
 	cases := []struct {
 		k8sRs         batch.JobList
@@ -91,7 +91,7 @@ func TestGetJobListFromChannels(t *testing.T) {
 					},
 					Spec: batch.JobSpec{
 						Selector:    &metaV1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						Completions: &jobCompletions,
+						Completions: &completions,
 					},
 					Status: batch.JobStatus{
 						Active: 7,
@@ -106,7 +106,8 @@ func TestGetJobListFromChannels(t *testing.T) {
 							CreationTimestamp: metaV1.Unix(111, 222),
 						},
 						Spec: batch.JobSpec{
-							Selector: &metaV1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
+							Selector:    &metaV1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
+							Completions: &completions,
 						},
 						Status: batch.JobStatus{
 							Active: 7,
@@ -158,7 +159,7 @@ func TestGetJobListFromChannels(t *testing.T) {
 					TypeMeta: api.TypeMeta{Kind: api.ResourceKindJob},
 					Pods: common.PodInfo{
 						Current:  7,
-						Desired:  21,
+						Desired:  &completions,
 						Failed:   2,
 						Warnings: []common.Event{},
 					},
@@ -172,7 +173,7 @@ func TestGetJobListFromChannels(t *testing.T) {
 					TypeMeta: api.TypeMeta{Kind: api.ResourceKindJob},
 					Pods: common.PodInfo{
 						Current:  7,
-						Desired:  0,
+						Desired:  &completions,
 						Failed:   2,
 						Warnings: []common.Event{},
 					},