From 68b2ac517aa3fc5bf067001345c4e5990cf02257 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Thu, 27 Dec 2018 16:13:49 +0100 Subject: [PATCH 01/31] capacity controller: include reporting in capacity target This is still incomplete: container entries should be aggregated when processing container states. --- pkg/apis/shipper/v1alpha1/types.go | 29 +++ .../capacity/capacity_controller.go | 28 +++ pkg/controller/capacity/reporting.go | 196 ++++++++++++++++++ 3 files changed, 253 insertions(+) create mode 100644 pkg/controller/capacity/reporting.go diff --git a/pkg/apis/shipper/v1alpha1/types.go b/pkg/apis/shipper/v1alpha1/types.go index 801f1e360..006619692 100644 --- a/pkg/apis/shipper/v1alpha1/types.go +++ b/pkg/apis/shipper/v1alpha1/types.go @@ -336,12 +336,41 @@ type CapacityTargetStatus struct { Clusters []ClusterCapacityStatus `json:"clusters,omitempty"` } +type ClusterCapacityReportContainerBreakdownExample struct { + Pod string `json:"pod"` +} + +type ClusterCapacityReportContainerBreakdown struct { + Count uint32 `json:"count"` + Example ClusterCapacityReportContainerBreakdownExample `json:"example"` + Name string `json:"name"` + Reason string `json:"reason,omitempty"` + Type string `json:"type"` +} + +type ClusterCapacityReportBreakdown struct { + Containers []ClusterCapacityReportContainerBreakdown `json:"containers,omitempty"` + Reason string `json:"reason,omitempty"` + Status string `json:"status"` + Type string `json:"type"` +} + +type ClusterCapacityReportOwner struct { + Name string `json:"name"` +} + +type ClusterCapacityReport struct { + Owner ClusterCapacityReportOwner `json:"owner"` + Breakdown []ClusterCapacityReportBreakdown `json:"breakdown,omitempty"` +} + type ClusterCapacityStatus struct { Name string `json:"name"` AvailableReplicas int32 `json:"availableReplicas"` AchievedPercent int32 `json:"achievedPercent"` SadPods []PodStatus `json:"sadPods,omitempty"` Conditions []ClusterCapacityCondition `json:"conditions,omitempty"` + Reports []ClusterCapacityReport `json:"reports,omitempty"` } type ClusterConditionType string diff --git a/pkg/controller/capacity/capacity_controller.go b/pkg/controller/capacity/capacity_controller.go index 326f7ac38..000b9c45b 100644 --- a/pkg/controller/capacity/capacity_controller.go +++ b/pkg/controller/capacity/capacity_controller.go @@ -205,6 +205,8 @@ func (c *Controller) capacityTargetSyncHandler(key string) bool { ct.Status.Clusters = append(ct.Status.Clusters, *clusterStatus) } + clusterStatus.Reports = []shipper.ClusterCapacityReport{} + // all the below functions add conditions to the clusterStatus as they do // their business, hence we're passing them a pointer. targetDeployment, err := c.findTargetDeploymentForClusterSpec(clusterSpec, targetNamespace, selector, clusterStatus) @@ -235,6 +237,11 @@ func (c *Controller) capacityTargetSyncHandler(key string) bool { clusterStatus.SadPods = sadPods + report, err := c.getReport(targetDeployment, clusterStatus) + if err == nil { + clusterStatus.Reports = append(clusterStatus.Reports, *report) + } + if len(sadPods) > 0 { continue } @@ -337,6 +344,27 @@ func (c *Controller) getSadPods(targetDeployment *appsv1.Deployment, clusterStat return sadPods, nil } +func (c *Controller) getReport(targetDeployment *appsv1.Deployment, clusterStatus *shipper.ClusterCapacityStatus) (*shipper.ClusterCapacityReport, error) { + targetClusterInformer, clusterErr := c.clusterClientStore.GetInformerFactory(clusterStatus.Name) + if clusterErr != nil { + // Not sure if each method should report operational conditions for + // the cluster it is operating on. + return nil, clusterErr + } + + selector := labels.Set(targetDeployment.Spec.Template.Labels).AsSelector() + podsList, clusterErr := targetClusterInformer.Core().V1().Pods().Lister().Pods(targetDeployment.Namespace).List(selector) + if clusterErr != nil { + return nil, clusterErr + } + + containerStateEntries := buildContainerStateEntries(podsList) + conditionSummaries := summarizeContainerStatesByCondition(containerStateEntries) + report := buildReport(targetDeployment.Name, conditionSummaries) + + return report, nil +} + func (c *Controller) findTargetDeploymentForClusterSpec(clusterSpec shipper.ClusterCapacityTarget, targetNamespace string, selector labels.Selector, clusterStatus *shipper.ClusterCapacityStatus) (*appsv1.Deployment, error) { targetClusterInformer, clusterErr := c.clusterClientStore.GetInformerFactory(clusterSpec.Name) if clusterErr != nil { diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go new file mode 100644 index 000000000..a142debb0 --- /dev/null +++ b/pkg/controller/capacity/reporting.go @@ -0,0 +1,196 @@ +package capacity + +import ( + core_v1 "k8s.io/api/core/v1" + "strings" + + shipper_v1alpha1 "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" +) + +type containerState struct { + cluster string + owner string + pod string + + conditionType *string + conditionStatus *string + conditionReason *string + + containerName *string + containerStateType *string + containerStateReason *string + containerStateMessage *string +} + +func ptr2string(v *string) string { + if v == nil { + return "" + } + return *v +} + +func string2ptr(v string) *string { + return &v +} + +func getContainerStateField(c core_v1.ContainerState, f string) string { + if c.Running != nil { + switch f { + case "type": + return "Running" + case "reason", "message": + return "" + } + } else if c.Waiting != nil { + switch f { + case "type": + return "Waiting" + case "reason": + return c.Waiting.Reason + case "message": + return c.Waiting.Message + } + } else if c.Terminated != nil { + switch f { + case "type": + return "Terminated" + case "reason": + return c.Terminated.Reason + case "message": + return c.Terminated.Message + } + } else { + return "" + } + panic("Shouldn't get in here") +} + +func buildContainerStateEntries(podsList []*core_v1.Pod) []containerState { + containerStates := make([]containerState, 0, 0) + + for _, pod := range podsList { + state := containerState{ + cluster: "microk8s", + owner: "shipper-deployment", + pod: pod.Name, + } + + for _, cond := range pod.Status.Conditions { + state.conditionType = string2ptr(string(cond.Type)) + state.conditionStatus = string2ptr(string(cond.Status)) + state.conditionReason = string2ptr(string(cond.Reason)) + + for _, containerStatus := range pod.Status.ContainerStatuses { + state.containerName = string2ptr(containerStatus.Name) + state.containerStateType = string2ptr(getContainerStateField(containerStatus.State, "type")) + state.containerStateReason = string2ptr(getContainerStateField(containerStatus.State, "reason")) + state.containerStateMessage = string2ptr(getContainerStateField(containerStatus.State, "message")) + + containerStates = append(containerStates, state) + } + } + } + + return containerStates +} + +type conditionSummary struct { + containers map[string]shipper_v1alpha1.ClusterCapacityReportContainerBreakdown + reason string + status string + typ string +} + +func buildKey(args ...string) string { + validArgs := make([]string, 0) + + for _, v := range args { + if len(v) == 0 { + break + } + validArgs = append(validArgs, v) + } + + return strings.Join(validArgs, "/") +} + +func summarizeContainerStateByCondition(conditionSummaries map[string]conditionSummary, state containerState) { + + conditionSummaryKey := buildKey( + state.cluster, + ptr2string(state.conditionType), + ptr2string(state.conditionStatus), + ptr2string(state.conditionReason), + ) + + containerStateKey := buildKey( + state.cluster, + ptr2string(state.containerName), + ptr2string(state.containerStateType), + ptr2string(state.containerStateReason), + ) + + if summary, ok := conditionSummaries[conditionSummaryKey]; !ok { + + containerStates := make(map[string]shipper_v1alpha1.ClusterCapacityReportContainerBreakdown) + + containerStates[containerStateKey] = shipper_v1alpha1.ClusterCapacityReportContainerBreakdown{ + Count: 1, + Example: shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample{Pod: state.pod}, + Name: ptr2string(state.containerName), + Reason: ptr2string(state.containerStateReason), + Type: ptr2string(state.containerStateType), + } + + conditionSummaries[conditionSummaryKey] = conditionSummary{ + containers: containerStates, + status: ptr2string(state.conditionStatus), + reason: ptr2string(state.conditionReason), + typ: ptr2string(state.conditionType), + } + } else { + if existingState, ok := summary.containers[containerStateKey]; !ok { + summary.containers[containerStateKey] = shipper_v1alpha1.ClusterCapacityReportContainerBreakdown{ + Count: 1, + Example: shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample{Pod: state.pod}, + Name: ptr2string(state.containerName), + Reason: ptr2string(state.containerStateReason), + Type: ptr2string(state.containerStateType), + } + } else { + existingState.Count = existingState.Count + 1 + summary.containers[containerStateKey] = existingState + } + } +} + +func summarizeContainerStatesByCondition(containerStates []containerState) map[string]conditionSummary { + conditionSummaries := make(map[string]conditionSummary) + for _, state := range containerStates { + summarizeContainerStateByCondition(conditionSummaries, state) + } + return conditionSummaries +} + +func buildReport(ownerName string, conditionSummaries map[string]conditionSummary) *shipper_v1alpha1.ClusterCapacityReport { + report := &shipper_v1alpha1.ClusterCapacityReport{ + Owner: shipper_v1alpha1.ClusterCapacityReportOwner{Name: ownerName}, + Breakdown: []shipper_v1alpha1.ClusterCapacityReportBreakdown{}, + } + + for _, cond := range conditionSummaries { + breakdown := shipper_v1alpha1.ClusterCapacityReportBreakdown{ + Type: cond.typ, + Status: cond.status, + Reason: cond.reason, + } + + for _, container := range cond.containers { + breakdown.Containers = append(breakdown.Containers, container) + } + + report.Breakdown = append(report.Breakdown, breakdown) + } + + return report +} From 54f12db5fab546490ab6b96bbf1b849a923d285a Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Thu, 27 Dec 2018 16:41:36 +0100 Subject: [PATCH 02/31] capacity controller: aggregate container states per state in report --- pkg/apis/shipper/v1alpha1/types.go | 8 +++- pkg/controller/capacity/reporting.go | 62 +++++++++++++++++++++------- 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/pkg/apis/shipper/v1alpha1/types.go b/pkg/apis/shipper/v1alpha1/types.go index 006619692..404886d08 100644 --- a/pkg/apis/shipper/v1alpha1/types.go +++ b/pkg/apis/shipper/v1alpha1/types.go @@ -340,14 +340,18 @@ type ClusterCapacityReportContainerBreakdownExample struct { Pod string `json:"pod"` } -type ClusterCapacityReportContainerBreakdown struct { +type ClusterCapacityReportContainerStateBreakdown struct { Count uint32 `json:"count"` Example ClusterCapacityReportContainerBreakdownExample `json:"example"` - Name string `json:"name"` Reason string `json:"reason,omitempty"` Type string `json:"type"` } +type ClusterCapacityReportContainerBreakdown struct { + Name string `json:"name"` + States []ClusterCapacityReportContainerStateBreakdown `json:"states"` +} + type ClusterCapacityReportBreakdown struct { Containers []ClusterCapacityReportContainerBreakdown `json:"containers,omitempty"` Reason string `json:"reason,omitempty"` diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index a142debb0..1b291feb9 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -94,8 +94,16 @@ func buildContainerStateEntries(podsList []*core_v1.Pod) []containerState { return containerStates } +type containerStateSummary struct { + count uint32 + example shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample + name string + reason string + typ string +} + type conditionSummary struct { - containers map[string]shipper_v1alpha1.ClusterCapacityReportContainerBreakdown + containers map[string]containerStateSummary reason string status string typ string @@ -132,14 +140,14 @@ func summarizeContainerStateByCondition(conditionSummaries map[string]conditionS if summary, ok := conditionSummaries[conditionSummaryKey]; !ok { - containerStates := make(map[string]shipper_v1alpha1.ClusterCapacityReportContainerBreakdown) + containerStates := make(map[string]containerStateSummary) - containerStates[containerStateKey] = shipper_v1alpha1.ClusterCapacityReportContainerBreakdown{ - Count: 1, - Example: shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample{Pod: state.pod}, - Name: ptr2string(state.containerName), - Reason: ptr2string(state.containerStateReason), - Type: ptr2string(state.containerStateType), + containerStates[containerStateKey] = containerStateSummary{ + count: 1, + example: shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample{Pod: state.pod}, + name: ptr2string(state.containerName), + reason: ptr2string(state.containerStateReason), + typ: ptr2string(state.containerStateType), } conditionSummaries[conditionSummaryKey] = conditionSummary{ @@ -150,15 +158,15 @@ func summarizeContainerStateByCondition(conditionSummaries map[string]conditionS } } else { if existingState, ok := summary.containers[containerStateKey]; !ok { - summary.containers[containerStateKey] = shipper_v1alpha1.ClusterCapacityReportContainerBreakdown{ - Count: 1, - Example: shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample{Pod: state.pod}, - Name: ptr2string(state.containerName), - Reason: ptr2string(state.containerStateReason), - Type: ptr2string(state.containerStateType), + summary.containers[containerStateKey] = containerStateSummary{ + count: 1, + example: shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample{Pod: state.pod}, + name: ptr2string(state.containerName), + reason: ptr2string(state.containerStateReason), + typ: ptr2string(state.containerStateType), } } else { - existingState.Count = existingState.Count + 1 + existingState.count = existingState.count + 1 summary.containers[containerStateKey] = existingState } } @@ -186,7 +194,29 @@ func buildReport(ownerName string, conditionSummaries map[string]conditionSummar } for _, container := range cond.containers { - breakdown.Containers = append(breakdown.Containers, container) + var containerBreakdown *shipper_v1alpha1.ClusterCapacityReportContainerBreakdown + + for _, c := range breakdown.Containers { + if c.Name == container.name { + containerBreakdown = &c + break + } + } + + if containerBreakdown == nil { + containerBreakdown = &shipper_v1alpha1.ClusterCapacityReportContainerBreakdown{ + Name: container.name, + } + } + + containerBreakdown.States = append(containerBreakdown.States, shipper_v1alpha1.ClusterCapacityReportContainerStateBreakdown{ + Reason: container.reason, + Type: container.typ, + Count: container.count, + Example: container.example, + }) + + breakdown.Containers = append(breakdown.Containers, *containerBreakdown) } report.Breakdown = append(report.Breakdown, breakdown) From e7f066a01997e78deff17d84595a0830fb2bb10f Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Mon, 11 Feb 2019 17:18:01 +0100 Subject: [PATCH 03/31] capacity controller: add tests for releases with single and multiple pods --- pkg/apis/shipper/v1alpha1/types.go | 1 + .../capacity/capacity_controller.go | 64 +++--- .../capacity/capacity_controller_test.go | 211 +++++++++++++++++- pkg/controller/capacity/reporting.go | 18 +- 4 files changed, 254 insertions(+), 40 deletions(-) diff --git a/pkg/apis/shipper/v1alpha1/types.go b/pkg/apis/shipper/v1alpha1/types.go index 404886d08..03fe2b7cc 100644 --- a/pkg/apis/shipper/v1alpha1/types.go +++ b/pkg/apis/shipper/v1alpha1/types.go @@ -354,6 +354,7 @@ type ClusterCapacityReportContainerBreakdown struct { type ClusterCapacityReportBreakdown struct { Containers []ClusterCapacityReportContainerBreakdown `json:"containers,omitempty"` + Count uint32 `json:"count"` Reason string `json:"reason,omitempty"` Status string `json:"status"` Type string `json:"type"` diff --git a/pkg/controller/capacity/capacity_controller.go b/pkg/controller/capacity/capacity_controller.go index 000b9c45b..45d90aa35 100644 --- a/pkg/controller/capacity/capacity_controller.go +++ b/pkg/controller/capacity/capacity_controller.go @@ -25,7 +25,6 @@ import ( listers "github.com/bookingcom/shipper/pkg/client/listers/shipper/v1alpha1" "github.com/bookingcom/shipper/pkg/clusterclientstore" "github.com/bookingcom/shipper/pkg/conditions" - shippercontroller "github.com/bookingcom/shipper/pkg/controller" "github.com/bookingcom/shipper/pkg/util/replicas" ) @@ -192,21 +191,26 @@ func (c *Controller) capacityTargetSyncHandler(key string) bool { var clusterStatus *shipper.ClusterCapacityStatus var targetDeployment *appsv1.Deployment + if ct.Status.Clusters == nil { + ct.Status.Clusters = []shipper.ClusterCapacityStatus{} + } + for i, cs := range ct.Status.Clusters { if cs.Name == clusterSpec.Name { clusterStatus = &ct.Status.Clusters[i] + clusterStatus.Reports = []shipper.ClusterCapacityReport{} + ct.Status.Clusters = append(ct.Status.Clusters[:i], ct.Status.Clusters[i+1:]...) + break } } if clusterStatus == nil { clusterStatus = &shipper.ClusterCapacityStatus{ - Name: clusterSpec.Name, + Name: clusterSpec.Name, + Reports: []shipper.ClusterCapacityReport{}, } - ct.Status.Clusters = append(ct.Status.Clusters, *clusterStatus) } - clusterStatus.Reports = []shipper.ClusterCapacityReport{} - // all the below functions add conditions to the clusterStatus as they do // their business, hence we're passing them a pointer. targetDeployment, err := c.findTargetDeploymentForClusterSpec(clusterSpec, targetNamespace, selector, clusterStatus) @@ -230,35 +234,36 @@ func (c *Controller) capacityTargetSyncHandler(key string) bool { clusterStatus.AvailableReplicas = targetDeployment.Status.AvailableReplicas clusterStatus.AchievedPercent = c.calculatePercentageFromAmount(clusterSpec.TotalReplicaCount, clusterStatus.AvailableReplicas) - sadPods, err := c.getSadPods(targetDeployment, clusterStatus) - if err != nil { - continue - } - - clusterStatus.SadPods = sadPods report, err := c.getReport(targetDeployment, clusterStatus) if err == nil { clusterStatus.Reports = append(clusterStatus.Reports, *report) } - if len(sadPods) > 0 { + sadPods, err := c.getSadPods(targetDeployment, clusterStatus) + if err != nil { + ct.Status.Clusters = append(ct.Status.Clusters, *clusterStatus) continue } + clusterStatus.SadPods = sadPods - // If we've got here, the capacity target has no sad pods and there have been - // no errors, so set conditions to true. - clusterStatus.Conditions = conditions.SetCapacityCondition( - clusterStatus.Conditions, - shipper.ClusterConditionTypeReady, - corev1.ConditionTrue, - "", "") - clusterStatus.Conditions = conditions.SetCapacityCondition( - clusterStatus.Conditions, - shipper.ClusterConditionTypeOperational, - corev1.ConditionTrue, - "", - "") + if len(sadPods) == 0 { + // If we've got here, the capacity target has no sad pods and there have been + // no errors, so set conditions to true. + clusterStatus.Conditions = conditions.SetCapacityCondition( + clusterStatus.Conditions, + shipper.ClusterConditionTypeReady, + corev1.ConditionTrue, + "", "") + clusterStatus.Conditions = conditions.SetCapacityCondition( + clusterStatus.Conditions, + shipper.ClusterConditionTypeOperational, + corev1.ConditionTrue, + "", + "") + } + + ct.Status.Clusters = append(ct.Status.Clusters, *clusterStatus) } sort.Sort(byClusterName(ct.Status.Clusters)) @@ -269,15 +274,6 @@ func (c *Controller) capacityTargetSyncHandler(key string) bool { return true } - c.recorder.Eventf( - ct, - corev1.EventTypeNormal, - "CapacityTargetChanged", - "Set %q status to %v", - shippercontroller.MetaKey(ct), - ct.Status, - ) - return false } diff --git a/pkg/controller/capacity/capacity_controller_test.go b/pkg/controller/capacity/capacity_controller_test.go index 4bd5f5341..a3449bf53 100644 --- a/pkg/controller/capacity/capacity_controller_test.go +++ b/pkg/controller/capacity/capacity_controller_test.go @@ -30,7 +30,7 @@ func TestUpdatingCapacityTargetUpdatesDeployment(t *testing.T) { f := NewFixture(t) capacityTarget := newCapacityTarget(10, 50) - f.managementObjects = append(f.managementObjects, capacityTarget) + f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) deployment := newDeployment(0, 0) f.targetClusterObjects = append(f.targetClusterObjects, deployment) @@ -53,11 +53,203 @@ func TestUpdatingCapacityTargetUpdatesDeployment(t *testing.T) { f.runCapacityTargetSyncHandler() } +func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePod(t *testing.T) { + f := NewFixture(t) + + capacityTarget := newCapacityTarget(1, 100) + + deployment := newDeployment(1, 1) + podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) + podA := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-a", + Namespace: capacityTarget.GetNamespace(), + Labels: podLabels, + }, + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "app", + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, + }, + }, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + }, + }, + }, + } + f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA) + + c := shipper.ClusterCapacityReport{ + Owner: shipper.ClusterCapacityReportOwner{ + Name: "nginx", + }, + Breakdown: []shipper.ClusterCapacityReportBreakdown{ + { + Count: 1, + Type: string(corev1.PodReady), + Containers: []shipper.ClusterCapacityReportContainerBreakdown{ + { + Name: "app", + States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 1, + Type: "Running", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + }, + }, + }, + }, + }, + }, + } + + f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) + + capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ + Name: "minikube", + Reports: []shipper.ClusterCapacityReport{c}, + AchievedPercent: 100, + AvailableReplicas: 1, + Conditions: []shipper.ClusterCapacityCondition{ + {Type: shipper.ClusterConditionTypeOperational, Status: corev1.ConditionTrue}, + {Type: shipper.ClusterConditionTypeReady, Status: corev1.ConditionTrue}, + }, + }) + + updateAction := kubetesting.NewUpdateAction( + schema.GroupVersionResource{ + Group: shipper.SchemeGroupVersion.Group, + Version: shipper.SchemeGroupVersion.Version, + Resource: "capacitytargets", + }, + capacityTarget.GetNamespace(), + capacityTarget, + ) + + f.managementClusterActions = append(f.managementClusterActions, updateAction) + f.runCapacityTargetSyncHandler() + + // Calling the sync handler again with the updated capacity target object should yield the same results. + f.managementObjects = []runtime.Object{capacityTarget.DeepCopy()} + f.runCapacityTargetSyncHandler() +} + +func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePods(t *testing.T) { + f := NewFixture(t) + + capacityTarget := newCapacityTarget(2, 100) + + deployment := newDeployment(2, 2) + podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) + podA := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-a", + Namespace: capacityTarget.GetNamespace(), + Labels: podLabels, + }, + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "app", + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, + }, + }, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + }, + }, + }, + } + podB := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-b", + Namespace: capacityTarget.GetNamespace(), + Labels: podLabels, + }, + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "app", + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, + }, + }, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + }, + }, + }, + } + f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA, podB) + + c := shipper.ClusterCapacityReport{ + Owner: shipper.ClusterCapacityReportOwner{ + Name: "nginx", + }, + Breakdown: []shipper.ClusterCapacityReportBreakdown{ + { + Count: 2, + Type: string(corev1.PodReady), + Containers: []shipper.ClusterCapacityReportContainerBreakdown{ + { + Name: "app", + States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 2, + Type: "Running", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + }, + }, + }, + }, + }, + }, + } + + f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) + + capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ + Name: "minikube", + Reports: []shipper.ClusterCapacityReport{c}, + AchievedPercent: 100, + AvailableReplicas: 2, + Conditions: []shipper.ClusterCapacityCondition{ + {Type: shipper.ClusterConditionTypeOperational, Status: corev1.ConditionTrue}, + {Type: shipper.ClusterConditionTypeReady, Status: corev1.ConditionTrue}, + }, + }) + + updateAction := kubetesting.NewUpdateAction( + schema.GroupVersionResource{ + Group: shipper.SchemeGroupVersion.Group, + Version: shipper.SchemeGroupVersion.Version, + Resource: "capacitytargets", + }, + capacityTarget.GetNamespace(), + capacityTarget, + ) + + f.managementClusterActions = append(f.managementClusterActions, updateAction) + f.runCapacityTargetSyncHandler() + + // Calling the sync handler again with the updated capacity target object should yield the same results. + f.managementObjects = []runtime.Object{capacityTarget.DeepCopy()} + f.runCapacityTargetSyncHandler() +} + func TestUpdatingDeploymentsUpdatesTheCapacityTargetStatus(t *testing.T) { f := NewFixture(t) capacityTarget := newCapacityTarget(10, 50) - f.managementObjects = append(f.managementObjects, capacityTarget) + f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) deployment := newDeployment(5, 5) f.targetClusterObjects = append(f.targetClusterObjects, deployment) @@ -82,7 +274,7 @@ func TestSadPodsAreReflectedInCapacityTargetStatus(t *testing.T) { f := NewFixture(t) capacityTarget := newCapacityTarget(2, 100) - f.managementObjects = append(f.managementObjects, capacityTarget) + f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) deployment := newDeployment(2, 1) happyPod := createHappyPodForDeployment(deployment) @@ -196,6 +388,12 @@ func (f *fixture) expectCapacityTargetStatusUpdate(capacityTarget *shipper.Capac AchievedPercent: achievedPercent, Conditions: clusterConditions, SadPods: sadPods, + Reports: []shipper.ClusterCapacityReport{ + { + Owner: shipper.ClusterCapacityReportOwner{Name: "nginx"}, + Breakdown: []shipper.ClusterCapacityReportBreakdown{}, + }, + }, } capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, clusterStatus) @@ -234,7 +432,7 @@ func newCapacityTarget(totalReplicaCount, percent int32) *shipper.CapacityTarget Namespace: namespace, Labels: metaLabels, OwnerReferences: []metav1.OwnerReference{ - metav1.OwnerReference{ + { APIVersion: shipper.SchemeGroupVersion.String(), Kind: "Release", Name: "0.0.1", @@ -277,6 +475,11 @@ func newDeployment(replicas int32, availableReplicas int32) *appsv1.Deployment { Spec: appsv1.DeploymentSpec{ Replicas: &replicas, Selector: specSelector, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: metaLabels, + }, + }, }, Status: status, } diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index 1b291feb9..a18200402 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -1,9 +1,11 @@ package capacity import ( - core_v1 "k8s.io/api/core/v1" + "sort" "strings" + core_v1 "k8s.io/api/core/v1" + shipper_v1alpha1 "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" ) @@ -11,6 +13,7 @@ type containerState struct { cluster string owner string pod string + count uint32 conditionType *string conditionStatus *string @@ -68,6 +71,11 @@ func getContainerStateField(c core_v1.ContainerState, f string) string { func buildContainerStateEntries(podsList []*core_v1.Pod) []containerState { containerStates := make([]containerState, 0, 0) + // Sort pods list to offer a stable pod as example. + sort.Slice(podsList, func(i, j int) bool { + return podsList[i].Name < podsList[j].Name + }) + for _, pod := range podsList { state := containerState{ cluster: "microk8s", @@ -79,7 +87,7 @@ func buildContainerStateEntries(podsList []*core_v1.Pod) []containerState { state.conditionType = string2ptr(string(cond.Type)) state.conditionStatus = string2ptr(string(cond.Status)) state.conditionReason = string2ptr(string(cond.Reason)) - + state.count = state.count + 1 for _, containerStatus := range pod.Status.ContainerStatuses { state.containerName = string2ptr(containerStatus.Name) state.containerStateType = string2ptr(getContainerStateField(containerStatus.State, "type")) @@ -103,6 +111,7 @@ type containerStateSummary struct { } type conditionSummary struct { + count uint32 containers map[string]containerStateSummary reason string status string @@ -151,12 +160,14 @@ func summarizeContainerStateByCondition(conditionSummaries map[string]conditionS } conditionSummaries[conditionSummaryKey] = conditionSummary{ + count: 1, containers: containerStates, status: ptr2string(state.conditionStatus), reason: ptr2string(state.conditionReason), typ: ptr2string(state.conditionType), } } else { + summary.count = summary.count + 1 if existingState, ok := summary.containers[containerStateKey]; !ok { summary.containers[containerStateKey] = containerStateSummary{ count: 1, @@ -169,6 +180,8 @@ func summarizeContainerStateByCondition(conditionSummaries map[string]conditionS existingState.count = existingState.count + 1 summary.containers[containerStateKey] = existingState } + + conditionSummaries[conditionSummaryKey] = summary } } @@ -191,6 +204,7 @@ func buildReport(ownerName string, conditionSummaries map[string]conditionSummar Type: cond.typ, Status: cond.status, Reason: cond.reason, + Count: cond.count, } for _, container := range cond.containers { From e437039b7a49404f5c4db37d00419326cddd7289 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Mon, 11 Feb 2019 17:30:41 +0100 Subject: [PATCH 04/31] hack/update-codegen.sh --- .../shipper/v1alpha1/zz_generated.deepcopy.go | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/pkg/apis/shipper/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/shipper/v1alpha1/zz_generated.deepcopy.go index eb22dcd52..172e71bc0 100644 --- a/pkg/apis/shipper/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/shipper/v1alpha1/zz_generated.deepcopy.go @@ -344,6 +344,123 @@ func (in *ClusterCapacityCondition) DeepCopy() *ClusterCapacityCondition { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterCapacityReport) DeepCopyInto(out *ClusterCapacityReport) { + *out = *in + out.Owner = in.Owner + if in.Breakdown != nil { + in, out := &in.Breakdown, &out.Breakdown + *out = make([]ClusterCapacityReportBreakdown, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterCapacityReport. +func (in *ClusterCapacityReport) DeepCopy() *ClusterCapacityReport { + if in == nil { + return nil + } + out := new(ClusterCapacityReport) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterCapacityReportBreakdown) DeepCopyInto(out *ClusterCapacityReportBreakdown) { + *out = *in + if in.Containers != nil { + in, out := &in.Containers, &out.Containers + *out = make([]ClusterCapacityReportContainerBreakdown, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterCapacityReportBreakdown. +func (in *ClusterCapacityReportBreakdown) DeepCopy() *ClusterCapacityReportBreakdown { + if in == nil { + return nil + } + out := new(ClusterCapacityReportBreakdown) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterCapacityReportContainerBreakdown) DeepCopyInto(out *ClusterCapacityReportContainerBreakdown) { + *out = *in + if in.States != nil { + in, out := &in.States, &out.States + *out = make([]ClusterCapacityReportContainerStateBreakdown, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterCapacityReportContainerBreakdown. +func (in *ClusterCapacityReportContainerBreakdown) DeepCopy() *ClusterCapacityReportContainerBreakdown { + if in == nil { + return nil + } + out := new(ClusterCapacityReportContainerBreakdown) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterCapacityReportContainerBreakdownExample) DeepCopyInto(out *ClusterCapacityReportContainerBreakdownExample) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterCapacityReportContainerBreakdownExample. +func (in *ClusterCapacityReportContainerBreakdownExample) DeepCopy() *ClusterCapacityReportContainerBreakdownExample { + if in == nil { + return nil + } + out := new(ClusterCapacityReportContainerBreakdownExample) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterCapacityReportContainerStateBreakdown) DeepCopyInto(out *ClusterCapacityReportContainerStateBreakdown) { + *out = *in + out.Example = in.Example + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterCapacityReportContainerStateBreakdown. +func (in *ClusterCapacityReportContainerStateBreakdown) DeepCopy() *ClusterCapacityReportContainerStateBreakdown { + if in == nil { + return nil + } + out := new(ClusterCapacityReportContainerStateBreakdown) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterCapacityReportOwner) DeepCopyInto(out *ClusterCapacityReportOwner) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterCapacityReportOwner. +func (in *ClusterCapacityReportOwner) DeepCopy() *ClusterCapacityReportOwner { + if in == nil { + return nil + } + out := new(ClusterCapacityReportOwner) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterCapacityStatus) DeepCopyInto(out *ClusterCapacityStatus) { *out = *in @@ -361,6 +478,13 @@ func (in *ClusterCapacityStatus) DeepCopyInto(out *ClusterCapacityStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Reports != nil { + in, out := &in.Reports, &out.Reports + *out = make([]ClusterCapacityReport, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } From bc0b8c6cb7f28afdd4c97184d9e5a838aaad6309 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Tue, 12 Feb 2019 10:54:22 +0100 Subject: [PATCH 05/31] reporting: make linter happy and clean up --- pkg/controller/capacity/reporting.go | 87 ++++++++++++++++++---------- 1 file changed, 57 insertions(+), 30 deletions(-) diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index a18200402..d951ab4d3 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -1,6 +1,7 @@ package capacity import ( + "fmt" "sort" "strings" @@ -9,6 +10,14 @@ import ( shipper_v1alpha1 "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" ) +type ContainerStateField string + +const ( + ContainerStateFieldType ContainerStateField = "type" + ContainerStateFieldReason ContainerStateField = "reason" + ContainerStateFieldMessage ContainerStateField = "message" +) + type containerState struct { cluster string owner string @@ -36,40 +45,58 @@ func string2ptr(v string) *string { return &v } -func getContainerStateField(c core_v1.ContainerState, f string) string { +func getRunningContainerStateField(field ContainerStateField) string { + switch field { + case ContainerStateFieldType: + return "Running" + case ContainerStateFieldReason, ContainerStateFieldMessage: + return "" + default: + panic(fmt.Sprintf("Unknown field %s", field)) + } +} + +func getWaitingContainerStateField(stateWaiting *core_v1.ContainerStateWaiting, field ContainerStateField) string { + switch field { + case ContainerStateFieldType: + return "Waiting" + case ContainerStateFieldReason: + return stateWaiting.Reason + case ContainerStateFieldMessage: + return stateWaiting.Message + default: + panic(fmt.Sprintf("Unknown field %s", field)) + } +} + +func getTerminatedContainerStateField(stateTerminated *core_v1.ContainerStateTerminated, f ContainerStateField) string { + switch f { + case ContainerStateFieldType: + return "Terminated" + case ContainerStateFieldReason: + return stateTerminated.Reason + case ContainerStateFieldMessage: + return stateTerminated.Message + default: + panic(fmt.Sprintf("Unknown field %s", f)) + } +} + +func getContainerStateField(c core_v1.ContainerState, f ContainerStateField) string { if c.Running != nil { - switch f { - case "type": - return "Running" - case "reason", "message": - return "" - } + return getRunningContainerStateField(f) } else if c.Waiting != nil { - switch f { - case "type": - return "Waiting" - case "reason": - return c.Waiting.Reason - case "message": - return c.Waiting.Message - } + return getWaitingContainerStateField(c.Waiting, f) } else if c.Terminated != nil { - switch f { - case "type": - return "Terminated" - case "reason": - return c.Terminated.Reason - case "message": - return c.Terminated.Message - } - } else { - return "" + return getTerminatedContainerStateField(c.Terminated, f) } - panic("Shouldn't get in here") + + // TODO: f should be a constant somehow. + panic("Programmer error: should be one of 'type', 'reason' or 'message'") } func buildContainerStateEntries(podsList []*core_v1.Pod) []containerState { - containerStates := make([]containerState, 0, 0) + containerStates := make([]containerState, 0) // Sort pods list to offer a stable pod as example. sort.Slice(podsList, func(i, j int) bool { @@ -90,9 +117,9 @@ func buildContainerStateEntries(podsList []*core_v1.Pod) []containerState { state.count = state.count + 1 for _, containerStatus := range pod.Status.ContainerStatuses { state.containerName = string2ptr(containerStatus.Name) - state.containerStateType = string2ptr(getContainerStateField(containerStatus.State, "type")) - state.containerStateReason = string2ptr(getContainerStateField(containerStatus.State, "reason")) - state.containerStateMessage = string2ptr(getContainerStateField(containerStatus.State, "message")) + state.containerStateType = string2ptr(getContainerStateField(containerStatus.State, ContainerStateFieldType)) + state.containerStateReason = string2ptr(getContainerStateField(containerStatus.State, ContainerStateFieldReason)) + state.containerStateMessage = string2ptr(getContainerStateField(containerStatus.State, ContainerStateFieldMessage)) containerStates = append(containerStates, state) } From 7dc41b668997feab3c998f4b68b521b4333ce5df Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Tue, 12 Feb 2019 11:11:31 +0100 Subject: [PATCH 06/31] reporting: better error message --- pkg/controller/capacity/reporting.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index d951ab4d3..171f70c2b 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -91,8 +91,7 @@ func getContainerStateField(c core_v1.ContainerState, f ContainerStateField) str return getTerminatedContainerStateField(c.Terminated, f) } - // TODO: f should be a constant somehow. - panic("Programmer error: should be one of 'type', 'reason' or 'message'") + panic("Programmer error: a container state must be either Running, Waiting or Terminated.") } func buildContainerStateEntries(podsList []*core_v1.Pod) []containerState { From 06436a2f9dd9782df3e5915deb995bb153f7e8cd Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Tue, 12 Feb 2019 11:20:06 +0100 Subject: [PATCH 07/31] reporting: propagate cluster name for proper keying --- pkg/controller/capacity/capacity_controller.go | 2 +- pkg/controller/capacity/reporting.go | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/controller/capacity/capacity_controller.go b/pkg/controller/capacity/capacity_controller.go index 45d90aa35..e8af0f89e 100644 --- a/pkg/controller/capacity/capacity_controller.go +++ b/pkg/controller/capacity/capacity_controller.go @@ -354,7 +354,7 @@ func (c *Controller) getReport(targetDeployment *appsv1.Deployment, clusterStatu return nil, clusterErr } - containerStateEntries := buildContainerStateEntries(podsList) + containerStateEntries := buildContainerStateEntries(clusterStatus.Name, podsList) conditionSummaries := summarizeContainerStatesByCondition(containerStateEntries) report := buildReport(targetDeployment.Name, conditionSummaries) diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index 171f70c2b..d7fb7f9e3 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -20,7 +20,6 @@ const ( type containerState struct { cluster string - owner string pod string count uint32 @@ -94,7 +93,7 @@ func getContainerStateField(c core_v1.ContainerState, f ContainerStateField) str panic("Programmer error: a container state must be either Running, Waiting or Terminated.") } -func buildContainerStateEntries(podsList []*core_v1.Pod) []containerState { +func buildContainerStateEntries(clusterName string, podsList []*core_v1.Pod) []containerState { containerStates := make([]containerState, 0) // Sort pods list to offer a stable pod as example. @@ -104,8 +103,7 @@ func buildContainerStateEntries(podsList []*core_v1.Pod) []containerState { for _, pod := range podsList { state := containerState{ - cluster: "microk8s", - owner: "shipper-deployment", + cluster: clusterName, pod: pod.Name, } From dfeadb32c301236699c571288befbaf43614de32 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Tue, 12 Feb 2019 14:40:57 +0100 Subject: [PATCH 08/31] reporting: capacity target returns correct report with multiple pods with different pod conditions --- .../capacity/capacity_controller_test.go | 163 ++++++++++++++++++ pkg/controller/capacity/deployment.go | 5 + pkg/controller/capacity/reporting.go | 51 +++--- 3 files changed, 195 insertions(+), 24 deletions(-) diff --git a/pkg/controller/capacity/capacity_controller_test.go b/pkg/controller/capacity/capacity_controller_test.go index a3449bf53..645b3b607 100644 --- a/pkg/controller/capacity/capacity_controller_test.go +++ b/pkg/controller/capacity/capacity_controller_test.go @@ -2,6 +2,7 @@ package capacity import ( "fmt" + "sort" "testing" "time" @@ -245,6 +246,168 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePods(t *testin f.runCapacityTargetSyncHandler() } +func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDifferentConditions(t *testing.T) { + f := NewFixture(t) + + capacityTarget := newCapacityTarget(2, 100) + + deployment := newDeployment(2, 2) + podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) + podA := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-a", + Namespace: capacityTarget.GetNamespace(), + Labels: podLabels, + }, + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "app", + State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ContainerCreating"}}, + }, + }, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: corev1.ConditionFalse, + Reason: "ContainersNotReady", + }, + }, + }, + } + podB := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-b", + Namespace: capacityTarget.GetNamespace(), + Labels: podLabels, + }, + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "app", + State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "Completed"}}, + }, + }, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: corev1.ConditionFalse, + Reason: "ContainersNotReady", + }, + }, + }, + } + f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA, podB) + + c := shipper.ClusterCapacityReport{ + Owner: shipper.ClusterCapacityReportOwner{ + Name: "nginx", + }, + Breakdown: []shipper.ClusterCapacityReportBreakdown{ + { + Count: 2, + Type: string(corev1.PodReady), + Status: string(corev1.ConditionFalse), + Reason: "ContainersNotReady", + Containers: []shipper.ClusterCapacityReportContainerBreakdown{ + { + Name: "app", + States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 1, + Type: "Terminated", + Reason: "Completed", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-b", + }, + }, + { + Count: 1, + Type: "Waiting", + Reason: "ContainerCreating", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + }, + }, + }, + }, + }, + }, + } + + f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) + + capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ + Name: "minikube", + Reports: []shipper.ClusterCapacityReport{c}, + AchievedPercent: 100, + AvailableReplicas: 2, + Conditions: []shipper.ClusterCapacityCondition{ + {Type: shipper.ClusterConditionTypeReady, Status: corev1.ConditionFalse, Reason: conditions.PodsNotReady, Message: "there are 2 sad pods"}, + }, + SadPods: []shipper.PodStatus{ + { + Condition: corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionFalse, + Reason: "ContainersNotReady", + }, + Containers: []corev1.ContainerStatus{ + { + Name: "app", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Reason: "Completed", + }, + }, + }, + }, + Name: "pod-b", + }, + { + Condition: corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionFalse, + Reason: "ContainersNotReady", + }, + Containers: []corev1.ContainerStatus{ + { + Name: "app", + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{ + Reason: "ContainerCreating", + }, + }, + }, + }, + Name: "pod-a", + }, + }, + }) + + sort.Slice(capacityTarget.Status.Clusters[0].SadPods, func(i, j int) bool { + return capacityTarget.Status.Clusters[0].SadPods[i].Name < capacityTarget.Status.Clusters[0].SadPods[j].Name + }) + + updateAction := kubetesting.NewUpdateAction( + schema.GroupVersionResource{ + Group: shipper.SchemeGroupVersion.Group, + Version: shipper.SchemeGroupVersion.Version, + Resource: "capacitytargets", + }, + capacityTarget.GetNamespace(), + capacityTarget, + ) + + f.managementClusterActions = append(f.managementClusterActions, updateAction) + f.runCapacityTargetSyncHandler() + + //// Calling the sync handler again with the updated capacity target object should yield the same results. + //f.managementObjects = []runtime.Object{capacityTarget.DeepCopy()} + //f.runCapacityTargetSyncHandler() +} + func TestUpdatingDeploymentsUpdatesTheCapacityTargetStatus(t *testing.T) { f := NewFixture(t) diff --git a/pkg/controller/capacity/deployment.go b/pkg/controller/capacity/deployment.go index e5a7b0060..2dca43f87 100644 --- a/pkg/controller/capacity/deployment.go +++ b/pkg/controller/capacity/deployment.go @@ -3,6 +3,7 @@ package capacity import ( "fmt" "math" + "sort" "github.com/golang/glog" appsv1 "k8s.io/api/apps/v1" @@ -183,6 +184,10 @@ func (c Controller) getSadPodsForDeploymentOnCluster(deployment *appsv1.Deployme } } + sort.Slice(sadPods, func(i, j int) bool { + return sadPods[i].Name < sadPods[j].Name + }) + return len(pods), len(sadPods), sadPods, nil } diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index d7fb7f9e3..6ac438609 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -21,7 +21,6 @@ const ( type containerState struct { cluster string pod string - count uint32 conditionType *string conditionStatus *string @@ -111,7 +110,6 @@ func buildContainerStateEntries(clusterName string, podsList []*core_v1.Pod) []c state.conditionType = string2ptr(string(cond.Type)) state.conditionStatus = string2ptr(string(cond.Status)) state.conditionReason = string2ptr(string(cond.Reason)) - state.count = state.count + 1 for _, containerStatus := range pod.Status.ContainerStatuses { state.containerName = string2ptr(containerStatus.Name) state.containerStateType = string2ptr(getContainerStateField(containerStatus.State, ContainerStateFieldType)) @@ -127,15 +125,15 @@ func buildContainerStateEntries(clusterName string, podsList []*core_v1.Pod) []c } type containerStateSummary struct { - count uint32 - example shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample - name string - reason string - typ string + containerCount uint32 + example shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample + name string + reason string + typ string } type conditionSummary struct { - count uint32 + podCount uint32 containers map[string]containerStateSummary reason string status string @@ -176,35 +174,35 @@ func summarizeContainerStateByCondition(conditionSummaries map[string]conditionS containerStates := make(map[string]containerStateSummary) containerStates[containerStateKey] = containerStateSummary{ - count: 1, - example: shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample{Pod: state.pod}, - name: ptr2string(state.containerName), - reason: ptr2string(state.containerStateReason), - typ: ptr2string(state.containerStateType), + containerCount: 1, + example: shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample{Pod: state.pod}, + name: ptr2string(state.containerName), + reason: ptr2string(state.containerStateReason), + typ: ptr2string(state.containerStateType), } conditionSummaries[conditionSummaryKey] = conditionSummary{ - count: 1, + podCount: 1, containers: containerStates, status: ptr2string(state.conditionStatus), reason: ptr2string(state.conditionReason), typ: ptr2string(state.conditionType), } } else { - summary.count = summary.count + 1 if existingState, ok := summary.containers[containerStateKey]; !ok { summary.containers[containerStateKey] = containerStateSummary{ - count: 1, - example: shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample{Pod: state.pod}, - name: ptr2string(state.containerName), - reason: ptr2string(state.containerStateReason), - typ: ptr2string(state.containerStateType), + containerCount: 1, + example: shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample{Pod: state.pod}, + name: ptr2string(state.containerName), + reason: ptr2string(state.containerStateReason), + typ: ptr2string(state.containerStateType), } } else { - existingState.count = existingState.count + 1 + existingState.containerCount = existingState.containerCount + 1 summary.containers[containerStateKey] = existingState } + summary.podCount = summary.podCount + 1 conditionSummaries[conditionSummaryKey] = summary } } @@ -228,15 +226,16 @@ func buildReport(ownerName string, conditionSummaries map[string]conditionSummar Type: cond.typ, Status: cond.status, Reason: cond.reason, - Count: cond.count, + Count: cond.podCount, } for _, container := range cond.containers { var containerBreakdown *shipper_v1alpha1.ClusterCapacityReportContainerBreakdown - for _, c := range breakdown.Containers { + for i, c := range breakdown.Containers { if c.Name == container.name { containerBreakdown = &c + breakdown.Containers = append(breakdown.Containers[:i], breakdown.Containers[i+1:]...) break } } @@ -250,10 +249,14 @@ func buildReport(ownerName string, conditionSummaries map[string]conditionSummar containerBreakdown.States = append(containerBreakdown.States, shipper_v1alpha1.ClusterCapacityReportContainerStateBreakdown{ Reason: container.reason, Type: container.typ, - Count: container.count, + Count: container.containerCount, Example: container.example, }) + sort.Slice(containerBreakdown.States, func(i, j int) bool { + return containerBreakdown.States[i].Type < containerBreakdown.States[j].Type + }) + breakdown.Containers = append(breakdown.Containers, *containerBreakdown) } From 79e2fc2e3c1275f5aedaafd57f38eeecdb9cf5be Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Tue, 12 Feb 2019 16:36:22 +0100 Subject: [PATCH 09/31] reporting: make complex test case more realistic --- .../capacity/capacity_controller_test.go | 140 ++++++++++++++++-- pkg/controller/capacity/reporting.go | 12 +- 2 files changed, 141 insertions(+), 11 deletions(-) diff --git a/pkg/controller/capacity/capacity_controller_test.go b/pkg/controller/capacity/capacity_controller_test.go index 645b3b607..480ddf30f 100644 --- a/pkg/controller/capacity/capacity_controller_test.go +++ b/pkg/controller/capacity/capacity_controller_test.go @@ -249,9 +249,9 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePods(t *testin func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDifferentConditions(t *testing.T) { f := NewFixture(t) - capacityTarget := newCapacityTarget(2, 100) + capacityTarget := newCapacityTarget(3, 100) - deployment := newDeployment(2, 2) + deployment := newDeployment(3, 3) podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) podA := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -267,6 +267,14 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer }, }, Conditions: []corev1.PodCondition{ + { + Type: corev1.PodScheduled, + Status: corev1.ConditionTrue, + }, + { + Type: corev1.PodInitialized, + Status: corev1.ConditionTrue, + }, { Type: corev1.PodReady, Status: corev1.ConditionFalse, @@ -289,6 +297,14 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer }, }, Conditions: []corev1.PodCondition{ + { + Type: corev1.PodScheduled, + Status: corev1.ConditionTrue, + }, + { + Type: corev1.PodInitialized, + Status: corev1.ConditionTrue, + }, { Type: corev1.PodReady, Status: corev1.ConditionFalse, @@ -297,7 +313,37 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer }, }, } - f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA, podB) + podC := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-c", + Namespace: capacityTarget.GetNamespace(), + Labels: podLabels, + }, + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "app", + State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ContainerCreating"}}, + }, + }, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodScheduled, + Status: corev1.ConditionTrue, + }, + { + Type: corev1.PodInitialized, + Status: corev1.ConditionTrue, + }, + { + Type: corev1.PodReady, + Status: corev1.ConditionFalse, + Reason: "ContainersNotReady", + }, + }, + }, + } + f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA, podB, podC) c := shipper.ClusterCapacityReport{ Owner: shipper.ClusterCapacityReportOwner{ @@ -305,7 +351,63 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer }, Breakdown: []shipper.ClusterCapacityReportBreakdown{ { - Count: 2, + Count: 3, + Type: string(corev1.PodInitialized), + Status: string(corev1.ConditionTrue), + Containers: []shipper.ClusterCapacityReportContainerBreakdown{ + { + Name: "app", + States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 1, + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-b", + }, + Reason: "Completed", + Type: "Terminated", + }, + { + Count: 2, + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + Reason: "ContainerCreating", + Type: "Waiting", + }, + }, + }, + }, + }, + { + Count: 3, + Type: string(corev1.PodScheduled), + Status: string(corev1.ConditionTrue), + Containers: []shipper.ClusterCapacityReportContainerBreakdown{ + { + Name: "app", + States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 1, + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-b", + }, + Type: "Terminated", + Reason: "Completed", + }, + { + Count: 2, + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + Type: "Waiting", + Reason: "ContainerCreating", + }, + }, + }, + }, + }, + { + Count: 3, Type: string(corev1.PodReady), Status: string(corev1.ConditionFalse), Reason: "ContainersNotReady", @@ -322,7 +424,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer }, }, { - Count: 1, + Count: 2, Type: "Waiting", Reason: "ContainerCreating", Example: shipper.ClusterCapacityReportContainerBreakdownExample{ @@ -342,9 +444,9 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer Name: "minikube", Reports: []shipper.ClusterCapacityReport{c}, AchievedPercent: 100, - AvailableReplicas: 2, + AvailableReplicas: 3, Conditions: []shipper.ClusterCapacityCondition{ - {Type: shipper.ClusterConditionTypeReady, Status: corev1.ConditionFalse, Reason: conditions.PodsNotReady, Message: "there are 2 sad pods"}, + {Type: shipper.ClusterConditionTypeReady, Status: corev1.ConditionFalse, Reason: conditions.PodsNotReady, Message: "there are 3 sad pods"}, }, SadPods: []shipper.PodStatus{ { @@ -383,6 +485,24 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer }, Name: "pod-a", }, + { + Condition: corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionFalse, + Reason: "ContainersNotReady", + }, + Containers: []corev1.ContainerStatus{ + { + Name: "app", + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{ + Reason: "ContainerCreating", + }, + }, + }, + }, + Name: "pod-c", + }, }, }) @@ -403,9 +523,9 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer f.managementClusterActions = append(f.managementClusterActions, updateAction) f.runCapacityTargetSyncHandler() - //// Calling the sync handler again with the updated capacity target object should yield the same results. - //f.managementObjects = []runtime.Object{capacityTarget.DeepCopy()} - //f.runCapacityTargetSyncHandler() + // Calling the sync handler again with the updated capacity target object should yield the same results. + f.managementObjects = []runtime.Object{capacityTarget.DeepCopy()} + f.runCapacityTargetSyncHandler() } func TestUpdatingDeploymentsUpdatesTheCapacityTargetStatus(t *testing.T) { diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index 6ac438609..eadbdb822 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -221,7 +221,17 @@ func buildReport(ownerName string, conditionSummaries map[string]conditionSummar Breakdown: []shipper_v1alpha1.ClusterCapacityReportBreakdown{}, } - for _, cond := range conditionSummaries { + var conditionSummaryKeys = []string{} + for k := range conditionSummaries { + conditionSummaryKeys = append(conditionSummaryKeys, k) + } + + sort.Slice(conditionSummaryKeys, func(i, j int) bool { + return conditionSummaries[conditionSummaryKeys[i]].typ < conditionSummaries[conditionSummaryKeys[j]].typ + }) + + for _, k := range conditionSummaryKeys { + cond := conditionSummaries[k] breakdown := shipper_v1alpha1.ClusterCapacityReportBreakdown{ Type: cond.typ, Status: cond.status, From f772c6ee70ca0893fce01742af30415f49eea0e2 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Tue, 12 Feb 2019 17:17:13 +0100 Subject: [PATCH 10/31] reporting: move sorting to another method --- pkg/controller/capacity/reporting.go | 34 ++++++++++++++++++---------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index eadbdb822..0e6ff4e73 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -215,23 +215,33 @@ func summarizeContainerStatesByCondition(containerStates []containerState) map[s return conditionSummaries } -func buildReport(ownerName string, conditionSummaries map[string]conditionSummary) *shipper_v1alpha1.ClusterCapacityReport { - report := &shipper_v1alpha1.ClusterCapacityReport{ - Owner: shipper_v1alpha1.ClusterCapacityReportOwner{Name: ownerName}, - Breakdown: []shipper_v1alpha1.ClusterCapacityReportBreakdown{}, - } +type conditionSummaryMap map[string]conditionSummary - var conditionSummaryKeys = []string{} - for k := range conditionSummaries { - conditionSummaryKeys = append(conditionSummaryKeys, k) +func (c conditionSummaryMap) SortedByKeyAsc() []conditionSummary { + var keys = []string{} + for k := range c { + keys = append(keys, k) } - sort.Slice(conditionSummaryKeys, func(i, j int) bool { - return conditionSummaries[conditionSummaryKeys[i]].typ < conditionSummaries[conditionSummaryKeys[j]].typ + sort.Slice(keys, func(i, j int) bool { + return c[keys[i]].typ < c[keys[j]].typ }) - for _, k := range conditionSummaryKeys { - cond := conditionSummaries[k] + var conds = []conditionSummary{} + for k := range keys { + conds = append(conds, c[keys[k]]) + } + + return conds +} + +func buildReport(ownerName string, conditionSummaries conditionSummaryMap) *shipper_v1alpha1.ClusterCapacityReport { + report := &shipper_v1alpha1.ClusterCapacityReport{ + Owner: shipper_v1alpha1.ClusterCapacityReportOwner{Name: ownerName}, + Breakdown: []shipper_v1alpha1.ClusterCapacityReportBreakdown{}, + } + + for _, cond := range conditionSummaries.SortedByKeyAsc() { breakdown := shipper_v1alpha1.ClusterCapacityReportBreakdown{ Type: cond.typ, Status: cond.status, From 81ae4a5d5bd52426223031396dddcd7462aa9da2 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Wed, 13 Feb 2019 15:39:10 +0100 Subject: [PATCH 11/31] reporting: create builders to aid testing --- .../capacity/capacity_controller_test.go | 514 ++++++------------ pkg/controller/capacity/report_builder.go | 132 +++++ pkg/controller/capacity/utils_test.go | 72 +++ 3 files changed, 385 insertions(+), 333 deletions(-) create mode 100644 pkg/controller/capacity/report_builder.go create mode 100644 pkg/controller/capacity/utils_test.go diff --git a/pkg/controller/capacity/capacity_controller_test.go b/pkg/controller/capacity/capacity_controller_test.go index 480ddf30f..1de284d0f 100644 --- a/pkg/controller/capacity/capacity_controller_test.go +++ b/pkg/controller/capacity/capacity_controller_test.go @@ -61,53 +61,47 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePod(t *testing.T deployment := newDeployment(1, 1) podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) - podA := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod-a", - Namespace: capacityTarget.GetNamespace(), - Labels: podLabels, - }, - Status: corev1.PodStatus{ - ContainerStatuses: []corev1.ContainerStatus{ - { - Name: "app", - State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, - }, - }, - Conditions: []corev1.PodCondition{ - { - Type: corev1.PodReady, - }, - }, - }, - } + + podA := newPodBuilder("pod-a", deployment.GetNamespace(), podLabels). + AddContainerStatus("app", corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionTrue}). + AddPodCondition(corev1.PodCondition{Type: "ContainersReady", Status: corev1.ConditionTrue}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionTrue}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodScheduled, Status: corev1.ConditionTrue}). + Build() + f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA) - c := shipper.ClusterCapacityReport{ - Owner: shipper.ClusterCapacityReportOwner{ - Name: "nginx", - }, - Breakdown: []shipper.ClusterCapacityReportBreakdown{ - { - Count: 1, - Type: string(corev1.PodReady), - Containers: []shipper.ClusterCapacityReportContainerBreakdown{ - { - Name: "app", - States: []shipper.ClusterCapacityReportContainerStateBreakdown{ - { - Count: 1, - Type: "Running", - Example: shipper.ClusterCapacityReportContainerBreakdownExample{ - Pod: "pod-a", - }, - }, - }, - }, - }, - }, - }, - } + c := newReportBuilder("nginx"). + AddBreakdown( + newBreakdownBuilder(1, "ContainersReady", string(corev1.ConditionTrue), ""). + AddContainerBreakdown( + newContainerBreakdownBuilder("app"). + AddState(1, "pod-a", "Running", ""). + Build()). + Build()). + AddBreakdown( + newBreakdownBuilder(1, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). + AddContainerBreakdown( + newContainerBreakdownBuilder("app"). + AddState(1, "pod-a", "Running", ""). + Build()). + Build()). + AddBreakdown( + newBreakdownBuilder(1, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). + AddContainerBreakdown( + newContainerBreakdownBuilder("app"). + AddState(1, "pod-a", "Running", ""). + Build()). + Build()). + AddBreakdown( + newBreakdownBuilder(1, "Ready", string(corev1.ConditionTrue), ""). + AddContainerBreakdown( + newContainerBreakdownBuilder("app"). + AddState(1, "pod-a", "Running", ""). + Build()). + Build()). + Build() f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) @@ -147,73 +141,55 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePods(t *testin deployment := newDeployment(2, 2) podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) - podA := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod-a", - Namespace: capacityTarget.GetNamespace(), - Labels: podLabels, - }, - Status: corev1.PodStatus{ - ContainerStatuses: []corev1.ContainerStatus{ - { - Name: "app", - State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, - }, - }, - Conditions: []corev1.PodCondition{ - { - Type: corev1.PodReady, - }, - }, - }, - } - podB := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod-b", - Namespace: capacityTarget.GetNamespace(), - Labels: podLabels, - }, - Status: corev1.PodStatus{ - ContainerStatuses: []corev1.ContainerStatus{ - { - Name: "app", - State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, - }, - }, - Conditions: []corev1.PodCondition{ - { - Type: corev1.PodReady, - }, - }, - }, - } + + podA := newPodBuilder("pod-a", deployment.GetNamespace(), podLabels). + AddContainerStatus("app", corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionTrue}). + AddPodCondition(corev1.PodCondition{Type: "ContainersReady", Status: corev1.ConditionTrue}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionTrue}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodScheduled, Status: corev1.ConditionTrue}). + Build() + + podB := newPodBuilder("pod-b", deployment.GetNamespace(), podLabels). + AddContainerStatus("app", corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionTrue}). + AddPodCondition(corev1.PodCondition{Type: "ContainersReady", Status: corev1.ConditionTrue}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionTrue}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodScheduled, Status: corev1.ConditionTrue}). + Build() + f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA, podB) - c := shipper.ClusterCapacityReport{ - Owner: shipper.ClusterCapacityReportOwner{ - Name: "nginx", - }, - Breakdown: []shipper.ClusterCapacityReportBreakdown{ - { - Count: 2, - Type: string(corev1.PodReady), - Containers: []shipper.ClusterCapacityReportContainerBreakdown{ - { - Name: "app", - States: []shipper.ClusterCapacityReportContainerStateBreakdown{ - { - Count: 2, - Type: "Running", - Example: shipper.ClusterCapacityReportContainerBreakdownExample{ - Pod: "pod-a", - }, - }, - }, - }, - }, - }, - }, - } + c := newReportBuilder("nginx"). + AddBreakdown( + newBreakdownBuilder(2, "ContainersReady", string(corev1.ConditionTrue), ""). + AddContainerBreakdown( + newContainerBreakdownBuilder("app"). + AddState(2, "pod-a", "Running", ""). + Build()). + Build()). + AddBreakdown( + newBreakdownBuilder(2, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). + AddContainerBreakdown( + newContainerBreakdownBuilder("app"). + AddState(2, "pod-a", "Running", ""). + Build()). + Build()). + AddBreakdown( + newBreakdownBuilder(2, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). + AddContainerBreakdown( + newContainerBreakdownBuilder("app"). + AddState(2, "pod-a", "Running", ""). + Build()). + Build()). + AddBreakdown( + newBreakdownBuilder(2, "Ready", string(corev1.ConditionTrue), ""). + AddContainerBreakdown( + newContainerBreakdownBuilder("app"). + AddState(2, "pod-a", "Running", ""). + Build()). + Build()). + Build() f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) @@ -253,192 +229,119 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer deployment := newDeployment(3, 3) podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) - podA := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod-a", - Namespace: capacityTarget.GetNamespace(), - Labels: podLabels, - }, - Status: corev1.PodStatus{ - ContainerStatuses: []corev1.ContainerStatus{ - { - Name: "app", - State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ContainerCreating"}}, - }, - }, - Conditions: []corev1.PodCondition{ - { - Type: corev1.PodScheduled, - Status: corev1.ConditionTrue, - }, - { - Type: corev1.PodInitialized, - Status: corev1.ConditionTrue, - }, - { - Type: corev1.PodReady, - Status: corev1.ConditionFalse, - Reason: "ContainersNotReady", - }, - }, - }, - } - podB := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod-b", - Namespace: capacityTarget.GetNamespace(), - Labels: podLabels, - }, - Status: corev1.PodStatus{ - ContainerStatuses: []corev1.ContainerStatus{ - { - Name: "app", - State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "Completed"}}, - }, + + podA := newPodBuilder("pod-a", deployment.GetNamespace(), podLabels). + AddContainerStatus("app", corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ContainerCreating"}}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionTrue}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionFalse, Reason: "ContainersNotReady"}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodScheduled, Status: corev1.ConditionTrue}). + Build() + + podB := newPodBuilder("pod-b", deployment.GetNamespace(), podLabels). + AddContainerStatus("app", corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ContainerCreating"}}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionTrue}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionFalse, Reason: "ContainersNotReady"}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodScheduled, Status: corev1.ConditionTrue}). + Build() + + podC := newPodBuilder("pod-c", deployment.GetNamespace(), podLabels). + AddContainerStatus("app", corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "Completed"}}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionTrue}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionFalse, Reason: "ContainersNotReady"}). + AddPodCondition(corev1.PodCondition{Type: corev1.PodScheduled, Status: corev1.ConditionTrue}). + Build() + + f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA, podB, podC) + + c := newReportBuilder("nginx"). + AddBreakdown( + newBreakdownBuilder(3, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). + AddContainerBreakdown( + newContainerBreakdownBuilder("app"). + AddState(2, "pod-a", "Waiting", "ContainerCreating"). + AddState(1, "pod-c", "Terminated", "Completed"). + Build()). + Build()). + AddBreakdown( + newBreakdownBuilder(3, string(corev1.PodReady), string(corev1.ConditionFalse), "ContainersNotReady"). + AddContainerBreakdown( + newContainerBreakdownBuilder("app"). + AddState(2, "pod-a", "Waiting", "ContainerCreating"). + AddState(1, "pod-c", "Terminated", "Completed"). + Build()). + Build()). + AddBreakdown( + newBreakdownBuilder(3, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). + AddContainerBreakdown( + newContainerBreakdownBuilder("app"). + AddState(2, "pod-a", "Waiting", "ContainerCreating"). + AddState(1, "pod-c", "Terminated", "Completed"). + Build()). + Build()). + Build() + + f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) + + sadPodsStatuses := []shipper.PodStatus{ + { + Condition: corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionFalse, + Reason: "ContainersNotReady", }, - Conditions: []corev1.PodCondition{ - { - Type: corev1.PodScheduled, - Status: corev1.ConditionTrue, - }, - { - Type: corev1.PodInitialized, - Status: corev1.ConditionTrue, - }, + Containers: []corev1.ContainerStatus{ { - Type: corev1.PodReady, - Status: corev1.ConditionFalse, - Reason: "ContainersNotReady", + Name: "app", + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{ + Reason: "ContainerCreating", + }, + }, }, }, + Name: "pod-a", }, - } - podC := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod-c", - Namespace: capacityTarget.GetNamespace(), - Labels: podLabels, - }, - Status: corev1.PodStatus{ - ContainerStatuses: []corev1.ContainerStatus{ - { - Name: "app", - State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ContainerCreating"}}, - }, + { + Condition: corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionFalse, + Reason: "ContainersNotReady", }, - Conditions: []corev1.PodCondition{ - { - Type: corev1.PodScheduled, - Status: corev1.ConditionTrue, - }, + Containers: []corev1.ContainerStatus{ { - Type: corev1.PodInitialized, - Status: corev1.ConditionTrue, - }, - { - Type: corev1.PodReady, - Status: corev1.ConditionFalse, - Reason: "ContainersNotReady", - }, - }, - }, - } - f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA, podB, podC) - - c := shipper.ClusterCapacityReport{ - Owner: shipper.ClusterCapacityReportOwner{ - Name: "nginx", - }, - Breakdown: []shipper.ClusterCapacityReportBreakdown{ - { - Count: 3, - Type: string(corev1.PodInitialized), - Status: string(corev1.ConditionTrue), - Containers: []shipper.ClusterCapacityReportContainerBreakdown{ - { - Name: "app", - States: []shipper.ClusterCapacityReportContainerStateBreakdown{ - { - Count: 1, - Example: shipper.ClusterCapacityReportContainerBreakdownExample{ - Pod: "pod-b", - }, - Reason: "Completed", - Type: "Terminated", - }, - { - Count: 2, - Example: shipper.ClusterCapacityReportContainerBreakdownExample{ - Pod: "pod-a", - }, - Reason: "ContainerCreating", - Type: "Waiting", - }, + Name: "app", + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{ + Reason: "ContainerCreating", }, }, }, }, - { - Count: 3, - Type: string(corev1.PodScheduled), - Status: string(corev1.ConditionTrue), - Containers: []shipper.ClusterCapacityReportContainerBreakdown{ - { - Name: "app", - States: []shipper.ClusterCapacityReportContainerStateBreakdown{ - { - Count: 1, - Example: shipper.ClusterCapacityReportContainerBreakdownExample{ - Pod: "pod-b", - }, - Type: "Terminated", - Reason: "Completed", - }, - { - Count: 2, - Example: shipper.ClusterCapacityReportContainerBreakdownExample{ - Pod: "pod-a", - }, - Type: "Waiting", - Reason: "ContainerCreating", - }, - }, - }, - }, - }, - { - Count: 3, - Type: string(corev1.PodReady), - Status: string(corev1.ConditionFalse), + Name: "pod-b", + }, + { + Condition: corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionFalse, Reason: "ContainersNotReady", - Containers: []shipper.ClusterCapacityReportContainerBreakdown{ - { - Name: "app", - States: []shipper.ClusterCapacityReportContainerStateBreakdown{ - { - Count: 1, - Type: "Terminated", - Reason: "Completed", - Example: shipper.ClusterCapacityReportContainerBreakdownExample{ - Pod: "pod-b", - }, - }, - { - Count: 2, - Type: "Waiting", - Reason: "ContainerCreating", - Example: shipper.ClusterCapacityReportContainerBreakdownExample{ - Pod: "pod-a", - }, - }, + }, + Containers: []corev1.ContainerStatus{ + { + Name: "app", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Reason: "Completed", }, }, }, }, + Name: "pod-c", }, } - f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) + sort.Slice(sadPodsStatuses, func(i, j int) bool { + return sadPodsStatuses[i].Name < sadPodsStatuses[j].Name + }) capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ Name: "minikube", @@ -448,62 +351,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer Conditions: []shipper.ClusterCapacityCondition{ {Type: shipper.ClusterConditionTypeReady, Status: corev1.ConditionFalse, Reason: conditions.PodsNotReady, Message: "there are 3 sad pods"}, }, - SadPods: []shipper.PodStatus{ - { - Condition: corev1.PodCondition{ - Type: corev1.PodReady, - Status: corev1.ConditionFalse, - Reason: "ContainersNotReady", - }, - Containers: []corev1.ContainerStatus{ - { - Name: "app", - State: corev1.ContainerState{ - Terminated: &corev1.ContainerStateTerminated{ - Reason: "Completed", - }, - }, - }, - }, - Name: "pod-b", - }, - { - Condition: corev1.PodCondition{ - Type: corev1.PodReady, - Status: corev1.ConditionFalse, - Reason: "ContainersNotReady", - }, - Containers: []corev1.ContainerStatus{ - { - Name: "app", - State: corev1.ContainerState{ - Waiting: &corev1.ContainerStateWaiting{ - Reason: "ContainerCreating", - }, - }, - }, - }, - Name: "pod-a", - }, - { - Condition: corev1.PodCondition{ - Type: corev1.PodReady, - Status: corev1.ConditionFalse, - Reason: "ContainersNotReady", - }, - Containers: []corev1.ContainerStatus{ - { - Name: "app", - State: corev1.ContainerState{ - Waiting: &corev1.ContainerStateWaiting{ - Reason: "ContainerCreating", - }, - }, - }, - }, - Name: "pod-c", - }, - }, + SadPods: sadPodsStatuses, }) sort.Slice(capacityTarget.Status.Clusters[0].SadPods, func(i, j int) bool { diff --git a/pkg/controller/capacity/report_builder.go b/pkg/controller/capacity/report_builder.go new file mode 100644 index 000000000..dfa0eb25e --- /dev/null +++ b/pkg/controller/capacity/report_builder.go @@ -0,0 +1,132 @@ +package capacity + +import ( + "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" + "sort" +) + +type reportBuilder struct { + ownerName string + breakdowns []v1alpha1.ClusterCapacityReportBreakdown +} + +func newReportBuilder(ownerName string) *reportBuilder { + return &reportBuilder{ownerName: ownerName} +} + +func (c *reportBuilder) AddBreakdown(breakdown v1alpha1.ClusterCapacityReportBreakdown) *reportBuilder { + c.breakdowns = append(c.breakdowns, breakdown) + return c +} + +func (c *reportBreakdownBuilder) AddContainerBreakdown(container v1alpha1.ClusterCapacityReportContainerBreakdown) *reportBreakdownBuilder { + c.containers = append(c.containers, container) + return c +} + +func (c *reportBuilder) Build() v1alpha1.ClusterCapacityReport { + + sort.Slice(c.breakdowns, func(i, j int) bool { + return c.breakdowns[i].Type < c.breakdowns[j].Type + }) + + report := v1alpha1.ClusterCapacityReport{ + Owner: v1alpha1.ClusterCapacityReportOwner{ + Name: c.ownerName, + }, + Breakdown: c.breakdowns, + } + return report +} + +type reportBreakdownBuilder struct { + podCount uint32 + podConditionType string + podConditionStatus string + name string + podConditionReason string + states []v1alpha1.ClusterCapacityReportContainerStateBreakdown + containers []v1alpha1.ClusterCapacityReportContainerBreakdown +} + +func newBreakdownBuilder( + podCount uint32, + podConditionType string, + podConditionStatus string, + podConditionReason string, +) *reportBreakdownBuilder { + return &reportBreakdownBuilder{ + podCount: podCount, + podConditionType: podConditionType, + podConditionStatus: podConditionStatus, + podConditionReason: podConditionReason, + } +} + +func (c *reportBreakdownBuilder) SetName(name string) *reportBreakdownBuilder { + c.name = name + return c +} + +func (c *reportBreakdownBuilder) AddState( + count uint32, + podExampleName string, + podConditionType, + podConditionStatus string, +) *reportBreakdownBuilder { + c.states = append(c.states, v1alpha1.ClusterCapacityReportContainerStateBreakdown{ + Count: count, + Type: podConditionType, + Example: v1alpha1.ClusterCapacityReportContainerBreakdownExample{ + Pod: podExampleName, + }, + }) + return c +} + +func (c *reportBreakdownBuilder) Build() v1alpha1.ClusterCapacityReportBreakdown { + return v1alpha1.ClusterCapacityReportBreakdown{ + Type: c.podConditionType, + Status: c.podConditionStatus, + Count: c.podCount, + Reason: c.podConditionReason, + Containers: c.containers, + } +} + +type containerBreakdownBuilder struct { + containerName string + states []v1alpha1.ClusterCapacityReportContainerStateBreakdown +} + +func newContainerBreakdownBuilder(containerName string) *containerBreakdownBuilder { + return &containerBreakdownBuilder{containerName: containerName} +} + +func (c *containerBreakdownBuilder) AddState( + containerCount uint32, + podExampleName string, + containerConditionType string, + containerConditionReason string, +) *containerBreakdownBuilder { + c.states = append(c.states, v1alpha1.ClusterCapacityReportContainerStateBreakdown{ + Count: containerCount, + Type: containerConditionType, + Reason: containerConditionReason, + Example: v1alpha1.ClusterCapacityReportContainerBreakdownExample{ + Pod: podExampleName, + }, + }) + return c +} + +func (c *containerBreakdownBuilder) Build() v1alpha1.ClusterCapacityReportContainerBreakdown { + sort.Slice(c.states, func(i, j int) bool { + return c.states[i].Type < c.states[j].Type + }) + + return v1alpha1.ClusterCapacityReportContainerBreakdown{ + Name: c.containerName, + States: c.states, + } +} diff --git a/pkg/controller/capacity/utils_test.go b/pkg/controller/capacity/utils_test.go new file mode 100644 index 000000000..b374c2813 --- /dev/null +++ b/pkg/controller/capacity/utils_test.go @@ -0,0 +1,72 @@ +package capacity + +import ( + "sort" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type PodBuilder struct { + podName string + podNamespace string + podLabels map[string]string + containerStatuses []corev1.ContainerStatus + podConditions []corev1.PodCondition +} + +func newPodBuilder(podName string, podNamespace string, podLabels map[string]string) *PodBuilder { + return &PodBuilder{ + podName: podName, + podNamespace: podNamespace, + podLabels: podLabels, + } +} + +func (p *PodBuilder) SetName(name string) *PodBuilder { + p.podName = name + return p +} + +func (p *PodBuilder) SetNamespace(namespace string) *PodBuilder { + p.podNamespace = namespace + return p +} + +func (p *PodBuilder) SetLabels(labels map[string]string) *PodBuilder { + p.podLabels = labels + return p +} + +func (p *PodBuilder) AddContainerStatus(containerName string, containerState corev1.ContainerState) *PodBuilder { + p.containerStatuses = append(p.containerStatuses, corev1.ContainerStatus{Name: containerName, State: containerState}) + return p +} + +func (p *PodBuilder) AddPodCondition(cond corev1.PodCondition) *PodBuilder { + p.podConditions = append(p.podConditions, cond) + return p +} + +func (p *PodBuilder) Build() *corev1.Pod { + + sort.Slice(p.podConditions, func(i, j int) bool { + return p.podConditions[i].Type < p.podConditions[j].Type + }) + + sort.Slice(p.containerStatuses, func(i, j int) bool { + return p.containerStatuses[i].Name < p.containerStatuses[j].Name + }) + + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: p.podName, + Namespace: p.podNamespace, + Labels: p.podLabels, + }, + Status: corev1.PodStatus{ + ContainerStatuses: p.containerStatuses, + Conditions: p.podConditions, + }, + } +} From 1fe22744ec0c8a7062601630977b0e8d248e2f37 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Wed, 13 Feb 2019 16:53:26 +0100 Subject: [PATCH 12/31] reporting: use builders when constructing report --- .../capacity/capacity_controller_test.go | 6 +- pkg/controller/capacity/report_builder.go | 29 ++++++--- pkg/controller/capacity/reporting.go | 63 ++++++------------- 3 files changed, 42 insertions(+), 56 deletions(-) diff --git a/pkg/controller/capacity/capacity_controller_test.go b/pkg/controller/capacity/capacity_controller_test.go index 1de284d0f..295b0fdbc 100644 --- a/pkg/controller/capacity/capacity_controller_test.go +++ b/pkg/controller/capacity/capacity_controller_test.go @@ -107,7 +107,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePod(t *testing.T capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ Name: "minikube", - Reports: []shipper.ClusterCapacityReport{c}, + Reports: []shipper.ClusterCapacityReport{*c}, AchievedPercent: 100, AvailableReplicas: 1, Conditions: []shipper.ClusterCapacityCondition{ @@ -195,7 +195,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePods(t *testin capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ Name: "minikube", - Reports: []shipper.ClusterCapacityReport{c}, + Reports: []shipper.ClusterCapacityReport{*c}, AchievedPercent: 100, AvailableReplicas: 2, Conditions: []shipper.ClusterCapacityCondition{ @@ -345,7 +345,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ Name: "minikube", - Reports: []shipper.ClusterCapacityReport{c}, + Reports: []shipper.ClusterCapacityReport{*c}, AchievedPercent: 100, AvailableReplicas: 3, Conditions: []shipper.ClusterCapacityCondition{ diff --git a/pkg/controller/capacity/report_builder.go b/pkg/controller/capacity/report_builder.go index dfa0eb25e..2608ef1a1 100644 --- a/pkg/controller/capacity/report_builder.go +++ b/pkg/controller/capacity/report_builder.go @@ -24,7 +24,7 @@ func (c *reportBreakdownBuilder) AddContainerBreakdown(container v1alpha1.Cluste return c } -func (c *reportBuilder) Build() v1alpha1.ClusterCapacityReport { +func (c *reportBuilder) Build() *v1alpha1.ClusterCapacityReport { sort.Slice(c.breakdowns, func(i, j int) bool { return c.breakdowns[i].Type < c.breakdowns[j].Type @@ -36,7 +36,7 @@ func (c *reportBuilder) Build() v1alpha1.ClusterCapacityReport { }, Breakdown: c.breakdowns, } - return report + return &report } type reportBreakdownBuilder struct { @@ -94,33 +94,42 @@ func (c *reportBreakdownBuilder) Build() v1alpha1.ClusterCapacityReportBreakdown } } -type containerBreakdownBuilder struct { +type ContainerBreakdownBuilder struct { containerName string states []v1alpha1.ClusterCapacityReportContainerStateBreakdown } -func newContainerBreakdownBuilder(containerName string) *containerBreakdownBuilder { - return &containerBreakdownBuilder{containerName: containerName} +func newContainerBreakdownBuilder(containerName string) *ContainerBreakdownBuilder { + return &ContainerBreakdownBuilder{containerName: containerName} } -func (c *containerBreakdownBuilder) AddState( +func (c *ContainerBreakdownBuilder) AddState( containerCount uint32, podExampleName string, containerConditionType string, containerConditionReason string, -) *containerBreakdownBuilder { - c.states = append(c.states, v1alpha1.ClusterCapacityReportContainerStateBreakdown{ +) *ContainerBreakdownBuilder { + + breakdown := v1alpha1.ClusterCapacityReportContainerStateBreakdown{ Count: containerCount, Type: containerConditionType, Reason: containerConditionReason, Example: v1alpha1.ClusterCapacityReportContainerBreakdownExample{ Pod: podExampleName, }, - }) + } + + for _, s := range c.states { + if s == breakdown { + return c + } + } + + c.states = append(c.states, breakdown) return c } -func (c *containerBreakdownBuilder) Build() v1alpha1.ClusterCapacityReportContainerBreakdown { +func (c *ContainerBreakdownBuilder) Build() v1alpha1.ClusterCapacityReportContainerBreakdown { sort.Slice(c.states, func(i, j int) bool { return c.states[i].Type < c.states[j].Type }) diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index 0e6ff4e73..12dce297b 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -235,53 +235,30 @@ func (c conditionSummaryMap) SortedByKeyAsc() []conditionSummary { return conds } -func buildReport(ownerName string, conditionSummaries conditionSummaryMap) *shipper_v1alpha1.ClusterCapacityReport { - report := &shipper_v1alpha1.ClusterCapacityReport{ - Owner: shipper_v1alpha1.ClusterCapacityReportOwner{Name: ownerName}, - Breakdown: []shipper_v1alpha1.ClusterCapacityReportBreakdown{}, +type ContainerBreakdownBuilderMap map[string]*ContainerBreakdownBuilder + +func (c ContainerBreakdownBuilderMap) Get(containerName string) *ContainerBreakdownBuilder { + var cbBuilder *ContainerBreakdownBuilder + var ok bool + if cbBuilder, ok = c[containerName]; !ok { + cbBuilder = newContainerBreakdownBuilder(containerName) + c[containerName] = cbBuilder } + return cbBuilder +} +func buildReport(ownerName string, conditionSummaries conditionSummaryMap) *shipper_v1alpha1.ClusterCapacityReport { + reportBuilder := newReportBuilder(ownerName) + containerBreakdownBuilders := make(ContainerBreakdownBuilderMap) for _, cond := range conditionSummaries.SortedByKeyAsc() { - breakdown := shipper_v1alpha1.ClusterCapacityReportBreakdown{ - Type: cond.typ, - Status: cond.status, - Reason: cond.reason, - Count: cond.podCount, - } - + breakdownBuilder := newBreakdownBuilder(cond.podCount, cond.typ, cond.status, cond.reason) for _, container := range cond.containers { - var containerBreakdown *shipper_v1alpha1.ClusterCapacityReportContainerBreakdown - - for i, c := range breakdown.Containers { - if c.Name == container.name { - containerBreakdown = &c - breakdown.Containers = append(breakdown.Containers[:i], breakdown.Containers[i+1:]...) - break - } - } - - if containerBreakdown == nil { - containerBreakdown = &shipper_v1alpha1.ClusterCapacityReportContainerBreakdown{ - Name: container.name, - } - } - - containerBreakdown.States = append(containerBreakdown.States, shipper_v1alpha1.ClusterCapacityReportContainerStateBreakdown{ - Reason: container.reason, - Type: container.typ, - Count: container.containerCount, - Example: container.example, - }) - - sort.Slice(containerBreakdown.States, func(i, j int) bool { - return containerBreakdown.States[i].Type < containerBreakdown.States[j].Type - }) - - breakdown.Containers = append(breakdown.Containers, *containerBreakdown) + containerBreakdownBuilders.Get(container.name).AddState(container.containerCount, container.example.Pod, container.typ, container.reason) } - - report.Breakdown = append(report.Breakdown, breakdown) + for _, v := range containerBreakdownBuilders { + breakdownBuilder.AddContainerBreakdown(v.Build()) + } + reportBuilder.AddBreakdown(breakdownBuilder.Build()) } - - return report + return reportBuilder.Build() } From 511400157b0d9039a11cf63907545b2fbcd14ab5 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Thu, 14 Feb 2019 14:40:35 +0100 Subject: [PATCH 13/31] report: clean up --- .../capacity/builder/container_breakdown.go | 52 +++++++ .../builder/pod_condition_breakdown.go | 42 ++++++ pkg/controller/capacity/builder/report.go | 35 +++++ .../capacity/capacity_controller_test.go | 51 +++---- pkg/controller/capacity/report_builder.go | 141 ------------------ pkg/controller/capacity/reporting.go | 26 ++-- 6 files changed, 169 insertions(+), 178 deletions(-) create mode 100644 pkg/controller/capacity/builder/container_breakdown.go create mode 100644 pkg/controller/capacity/builder/pod_condition_breakdown.go create mode 100644 pkg/controller/capacity/builder/report.go delete mode 100644 pkg/controller/capacity/report_builder.go diff --git a/pkg/controller/capacity/builder/container_breakdown.go b/pkg/controller/capacity/builder/container_breakdown.go new file mode 100644 index 000000000..aa38ae3fa --- /dev/null +++ b/pkg/controller/capacity/builder/container_breakdown.go @@ -0,0 +1,52 @@ +package builder + +import ( + "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" + "sort" +) + +type ContainerStateBreakdown struct { + containerName string + states []v1alpha1.ClusterCapacityReportContainerStateBreakdown +} + +func NewContainerBreakdown(containerName string) *ContainerStateBreakdown { + return &ContainerStateBreakdown{containerName: containerName} +} + +func (c *ContainerStateBreakdown) AddState( + containerCount uint32, + podExampleName string, + containerConditionType string, + containerConditionReason string, +) *ContainerStateBreakdown { + + breakdown := v1alpha1.ClusterCapacityReportContainerStateBreakdown{ + Count: containerCount, + Type: containerConditionType, + Reason: containerConditionReason, + Example: v1alpha1.ClusterCapacityReportContainerBreakdownExample{ + Pod: podExampleName, + }, + } + + for _, s := range c.states { + if s == breakdown { + return c + } + } + + c.states = append(c.states, breakdown) + return c +} + +func (c *ContainerStateBreakdown) Build() v1alpha1.ClusterCapacityReportContainerBreakdown { + sort.Slice(c.states, func(i, j int) bool { + return c.states[i].Type < c.states[j].Type + }) + + return v1alpha1.ClusterCapacityReportContainerBreakdown{ + Name: c.containerName, + States: c.states, + } +} diff --git a/pkg/controller/capacity/builder/pod_condition_breakdown.go b/pkg/controller/capacity/builder/pod_condition_breakdown.go new file mode 100644 index 000000000..149433017 --- /dev/null +++ b/pkg/controller/capacity/builder/pod_condition_breakdown.go @@ -0,0 +1,42 @@ +package builder + +import "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" + +type PodConditionBreakdown struct { + podCount uint32 + podConditionType string + podConditionStatus string + name string + podConditionReason string + states []v1alpha1.ClusterCapacityReportContainerStateBreakdown + containers []v1alpha1.ClusterCapacityReportContainerBreakdown +} + +func NewPodConditionBreakdown( + podCount uint32, + podConditionType string, + podConditionStatus string, + podConditionReason string, +) *PodConditionBreakdown { + return &PodConditionBreakdown{ + podCount: podCount, + podConditionType: podConditionType, + podConditionStatus: podConditionStatus, + podConditionReason: podConditionReason, + } +} + +func (p *PodConditionBreakdown) AddContainerBreakdown(container v1alpha1.ClusterCapacityReportContainerBreakdown) *PodConditionBreakdown { + p.containers = append(p.containers, container) + return p +} + +func (p *PodConditionBreakdown) Build() v1alpha1.ClusterCapacityReportBreakdown { + return v1alpha1.ClusterCapacityReportBreakdown{ + Type: p.podConditionType, + Status: p.podConditionStatus, + Count: p.podCount, + Reason: p.podConditionReason, + Containers: p.containers, + } +} diff --git a/pkg/controller/capacity/builder/report.go b/pkg/controller/capacity/builder/report.go new file mode 100644 index 000000000..6b18e5daa --- /dev/null +++ b/pkg/controller/capacity/builder/report.go @@ -0,0 +1,35 @@ +package builder + +import ( + "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" + "sort" +) + +type Report struct { + ownerName string + breakdowns []v1alpha1.ClusterCapacityReportBreakdown +} + +func NewReport(ownerName string) *Report { + return &Report{ownerName: ownerName} +} + +func (r *Report) AddBreakdown(breakdown v1alpha1.ClusterCapacityReportBreakdown) *Report { + r.breakdowns = append(r.breakdowns, breakdown) + return r +} + +func (r *Report) Build() *v1alpha1.ClusterCapacityReport { + + sort.Slice(r.breakdowns, func(i, j int) bool { + return r.breakdowns[i].Type < r.breakdowns[j].Type + }) + + report := v1alpha1.ClusterCapacityReport{ + Owner: v1alpha1.ClusterCapacityReportOwner{ + Name: r.ownerName, + }, + Breakdown: r.breakdowns, + } + return &report +} diff --git a/pkg/controller/capacity/capacity_controller_test.go b/pkg/controller/capacity/capacity_controller_test.go index 295b0fdbc..3aabc931a 100644 --- a/pkg/controller/capacity/capacity_controller_test.go +++ b/pkg/controller/capacity/capacity_controller_test.go @@ -2,6 +2,7 @@ package capacity import ( "fmt" + "github.com/bookingcom/shipper/pkg/controller/capacity/builder" "sort" "testing" "time" @@ -72,32 +73,32 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePod(t *testing.T f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA) - c := newReportBuilder("nginx"). + c := builder.NewReport("nginx"). AddBreakdown( - newBreakdownBuilder(1, "ContainersReady", string(corev1.ConditionTrue), ""). + builder.NewPodConditionBreakdown(1, "ContainersReady", string(corev1.ConditionTrue), ""). AddContainerBreakdown( - newContainerBreakdownBuilder("app"). + builder.NewContainerBreakdown("app"). AddState(1, "pod-a", "Running", ""). Build()). Build()). AddBreakdown( - newBreakdownBuilder(1, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). + builder.NewPodConditionBreakdown(1, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). AddContainerBreakdown( - newContainerBreakdownBuilder("app"). + builder.NewContainerBreakdown("app"). AddState(1, "pod-a", "Running", ""). Build()). Build()). AddBreakdown( - newBreakdownBuilder(1, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). + builder.NewPodConditionBreakdown(1, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). AddContainerBreakdown( - newContainerBreakdownBuilder("app"). + builder.NewContainerBreakdown("app"). AddState(1, "pod-a", "Running", ""). Build()). Build()). AddBreakdown( - newBreakdownBuilder(1, "Ready", string(corev1.ConditionTrue), ""). + builder.NewPodConditionBreakdown(1, "Ready", string(corev1.ConditionTrue), ""). AddContainerBreakdown( - newContainerBreakdownBuilder("app"). + builder.NewContainerBreakdown("app"). AddState(1, "pod-a", "Running", ""). Build()). Build()). @@ -160,32 +161,32 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePods(t *testin f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA, podB) - c := newReportBuilder("nginx"). + c := builder.NewReport("nginx"). AddBreakdown( - newBreakdownBuilder(2, "ContainersReady", string(corev1.ConditionTrue), ""). + builder.NewPodConditionBreakdown(2, "ContainersReady", string(corev1.ConditionTrue), ""). AddContainerBreakdown( - newContainerBreakdownBuilder("app"). + builder.NewContainerBreakdown("app"). AddState(2, "pod-a", "Running", ""). Build()). Build()). AddBreakdown( - newBreakdownBuilder(2, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). + builder.NewPodConditionBreakdown(2, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). AddContainerBreakdown( - newContainerBreakdownBuilder("app"). + builder.NewContainerBreakdown("app"). AddState(2, "pod-a", "Running", ""). Build()). Build()). AddBreakdown( - newBreakdownBuilder(2, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). + builder.NewPodConditionBreakdown(2, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). AddContainerBreakdown( - newContainerBreakdownBuilder("app"). + builder.NewContainerBreakdown("app"). AddState(2, "pod-a", "Running", ""). Build()). Build()). AddBreakdown( - newBreakdownBuilder(2, "Ready", string(corev1.ConditionTrue), ""). + builder.NewPodConditionBreakdown(2, "Ready", string(corev1.ConditionTrue), ""). AddContainerBreakdown( - newContainerBreakdownBuilder("app"). + builder.NewContainerBreakdown("app"). AddState(2, "pod-a", "Running", ""). Build()). Build()). @@ -253,27 +254,27 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA, podB, podC) - c := newReportBuilder("nginx"). + c := builder.NewReport("nginx"). AddBreakdown( - newBreakdownBuilder(3, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). + builder.NewPodConditionBreakdown(3, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). AddContainerBreakdown( - newContainerBreakdownBuilder("app"). + builder.NewContainerBreakdown("app"). AddState(2, "pod-a", "Waiting", "ContainerCreating"). AddState(1, "pod-c", "Terminated", "Completed"). Build()). Build()). AddBreakdown( - newBreakdownBuilder(3, string(corev1.PodReady), string(corev1.ConditionFalse), "ContainersNotReady"). + builder.NewPodConditionBreakdown(3, string(corev1.PodReady), string(corev1.ConditionFalse), "ContainersNotReady"). AddContainerBreakdown( - newContainerBreakdownBuilder("app"). + builder.NewContainerBreakdown("app"). AddState(2, "pod-a", "Waiting", "ContainerCreating"). AddState(1, "pod-c", "Terminated", "Completed"). Build()). Build()). AddBreakdown( - newBreakdownBuilder(3, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). + builder.NewPodConditionBreakdown(3, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). AddContainerBreakdown( - newContainerBreakdownBuilder("app"). + builder.NewContainerBreakdown("app"). AddState(2, "pod-a", "Waiting", "ContainerCreating"). AddState(1, "pod-c", "Terminated", "Completed"). Build()). diff --git a/pkg/controller/capacity/report_builder.go b/pkg/controller/capacity/report_builder.go deleted file mode 100644 index 2608ef1a1..000000000 --- a/pkg/controller/capacity/report_builder.go +++ /dev/null @@ -1,141 +0,0 @@ -package capacity - -import ( - "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" - "sort" -) - -type reportBuilder struct { - ownerName string - breakdowns []v1alpha1.ClusterCapacityReportBreakdown -} - -func newReportBuilder(ownerName string) *reportBuilder { - return &reportBuilder{ownerName: ownerName} -} - -func (c *reportBuilder) AddBreakdown(breakdown v1alpha1.ClusterCapacityReportBreakdown) *reportBuilder { - c.breakdowns = append(c.breakdowns, breakdown) - return c -} - -func (c *reportBreakdownBuilder) AddContainerBreakdown(container v1alpha1.ClusterCapacityReportContainerBreakdown) *reportBreakdownBuilder { - c.containers = append(c.containers, container) - return c -} - -func (c *reportBuilder) Build() *v1alpha1.ClusterCapacityReport { - - sort.Slice(c.breakdowns, func(i, j int) bool { - return c.breakdowns[i].Type < c.breakdowns[j].Type - }) - - report := v1alpha1.ClusterCapacityReport{ - Owner: v1alpha1.ClusterCapacityReportOwner{ - Name: c.ownerName, - }, - Breakdown: c.breakdowns, - } - return &report -} - -type reportBreakdownBuilder struct { - podCount uint32 - podConditionType string - podConditionStatus string - name string - podConditionReason string - states []v1alpha1.ClusterCapacityReportContainerStateBreakdown - containers []v1alpha1.ClusterCapacityReportContainerBreakdown -} - -func newBreakdownBuilder( - podCount uint32, - podConditionType string, - podConditionStatus string, - podConditionReason string, -) *reportBreakdownBuilder { - return &reportBreakdownBuilder{ - podCount: podCount, - podConditionType: podConditionType, - podConditionStatus: podConditionStatus, - podConditionReason: podConditionReason, - } -} - -func (c *reportBreakdownBuilder) SetName(name string) *reportBreakdownBuilder { - c.name = name - return c -} - -func (c *reportBreakdownBuilder) AddState( - count uint32, - podExampleName string, - podConditionType, - podConditionStatus string, -) *reportBreakdownBuilder { - c.states = append(c.states, v1alpha1.ClusterCapacityReportContainerStateBreakdown{ - Count: count, - Type: podConditionType, - Example: v1alpha1.ClusterCapacityReportContainerBreakdownExample{ - Pod: podExampleName, - }, - }) - return c -} - -func (c *reportBreakdownBuilder) Build() v1alpha1.ClusterCapacityReportBreakdown { - return v1alpha1.ClusterCapacityReportBreakdown{ - Type: c.podConditionType, - Status: c.podConditionStatus, - Count: c.podCount, - Reason: c.podConditionReason, - Containers: c.containers, - } -} - -type ContainerBreakdownBuilder struct { - containerName string - states []v1alpha1.ClusterCapacityReportContainerStateBreakdown -} - -func newContainerBreakdownBuilder(containerName string) *ContainerBreakdownBuilder { - return &ContainerBreakdownBuilder{containerName: containerName} -} - -func (c *ContainerBreakdownBuilder) AddState( - containerCount uint32, - podExampleName string, - containerConditionType string, - containerConditionReason string, -) *ContainerBreakdownBuilder { - - breakdown := v1alpha1.ClusterCapacityReportContainerStateBreakdown{ - Count: containerCount, - Type: containerConditionType, - Reason: containerConditionReason, - Example: v1alpha1.ClusterCapacityReportContainerBreakdownExample{ - Pod: podExampleName, - }, - } - - for _, s := range c.states { - if s == breakdown { - return c - } - } - - c.states = append(c.states, breakdown) - return c -} - -func (c *ContainerBreakdownBuilder) Build() v1alpha1.ClusterCapacityReportContainerBreakdown { - sort.Slice(c.states, func(i, j int) bool { - return c.states[i].Type < c.states[j].Type - }) - - return v1alpha1.ClusterCapacityReportContainerBreakdown{ - Name: c.containerName, - States: c.states, - } -} diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index 12dce297b..1f4c87aef 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -2,6 +2,7 @@ package capacity import ( "fmt" + "github.com/bookingcom/shipper/pkg/controller/capacity/builder" "sort" "strings" @@ -235,27 +236,28 @@ func (c conditionSummaryMap) SortedByKeyAsc() []conditionSummary { return conds } -type ContainerBreakdownBuilderMap map[string]*ContainerBreakdownBuilder +type containerStateBreakdownBuilders map[string]*builder.ContainerStateBreakdown -func (c ContainerBreakdownBuilderMap) Get(containerName string) *ContainerBreakdownBuilder { - var cbBuilder *ContainerBreakdownBuilder +func (c containerStateBreakdownBuilders) Get(containerName string) *builder.ContainerStateBreakdown { + var b *builder.ContainerStateBreakdown var ok bool - if cbBuilder, ok = c[containerName]; !ok { - cbBuilder = newContainerBreakdownBuilder(containerName) - c[containerName] = cbBuilder + if b, ok = c[containerName]; !ok { + b = builder.NewContainerBreakdown(containerName) + c[containerName] = b } - return cbBuilder + return b } func buildReport(ownerName string, conditionSummaries conditionSummaryMap) *shipper_v1alpha1.ClusterCapacityReport { - reportBuilder := newReportBuilder(ownerName) - containerBreakdownBuilders := make(ContainerBreakdownBuilderMap) + reportBuilder := builder.NewReport(ownerName) + containerStateBreakdownBuilders := make(containerStateBreakdownBuilders) + for _, cond := range conditionSummaries.SortedByKeyAsc() { - breakdownBuilder := newBreakdownBuilder(cond.podCount, cond.typ, cond.status, cond.reason) + breakdownBuilder := builder.NewPodConditionBreakdown(cond.podCount, cond.typ, cond.status, cond.reason) for _, container := range cond.containers { - containerBreakdownBuilders.Get(container.name).AddState(container.containerCount, container.example.Pod, container.typ, container.reason) + containerStateBreakdownBuilders.Get(container.name).AddState(container.containerCount, container.example.Pod, container.typ, container.reason) } - for _, v := range containerBreakdownBuilders { + for _, v := range containerStateBreakdownBuilders { breakdownBuilder.AddContainerBreakdown(v.Build()) } reportBuilder.AddBreakdown(breakdownBuilder.Build()) From 378658a333e6dd6257b20560cda188cb4abb908f Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Thu, 14 Feb 2019 14:58:20 +0100 Subject: [PATCH 14/31] report: move types around --- pkg/controller/capacity/builder/report.go | 63 +++++++++++++++++++++- pkg/controller/capacity/reporting.go | 64 ++--------------------- 2 files changed, 65 insertions(+), 62 deletions(-) diff --git a/pkg/controller/capacity/builder/report.go b/pkg/controller/capacity/builder/report.go index 6b18e5daa..493d07b5a 100644 --- a/pkg/controller/capacity/builder/report.go +++ b/pkg/controller/capacity/builder/report.go @@ -1,8 +1,12 @@ package builder import ( - "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" + "fmt" "sort" + + core_v1 "k8s.io/api/core/v1" + + "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" ) type Report struct { @@ -33,3 +37,60 @@ func (r *Report) Build() *v1alpha1.ClusterCapacityReport { } return &report } + +func GetRunningContainerStateField(field ContainerStateField) string { + switch field { + case ContainerStateFieldType: + return "Running" + case ContainerStateFieldReason, ContainerStateFieldMessage: + return "" + default: + panic(fmt.Sprintf("Unknown field %s", field)) + } +} + +func GetWaitingContainerStateField(stateWaiting *core_v1.ContainerStateWaiting, field ContainerStateField) string { + switch field { + case ContainerStateFieldType: + return "Waiting" + case ContainerStateFieldReason: + return stateWaiting.Reason + case ContainerStateFieldMessage: + return stateWaiting.Message + default: + panic(fmt.Sprintf("Unknown field %s", field)) + } +} + +func GetTerminatedContainerStateField(stateTerminated *core_v1.ContainerStateTerminated, f ContainerStateField) string { + switch f { + case ContainerStateFieldType: + return "Terminated" + case ContainerStateFieldReason: + return stateTerminated.Reason + case ContainerStateFieldMessage: + return stateTerminated.Message + default: + panic(fmt.Sprintf("Unknown field %s", f)) + } +} + +func GetContainerStateField(c core_v1.ContainerState, f ContainerStateField) string { + if c.Running != nil { + return GetRunningContainerStateField(f) + } else if c.Waiting != nil { + return GetWaitingContainerStateField(c.Waiting, f) + } else if c.Terminated != nil { + return GetTerminatedContainerStateField(c.Terminated, f) + } + + panic("Programmer error: a container state must be either Running, Waiting or Terminated.") +} + +type ContainerStateField string + +const ( + ContainerStateFieldType ContainerStateField = "type" + ContainerStateFieldReason ContainerStateField = "reason" + ContainerStateFieldMessage ContainerStateField = "message" +) diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index 1f4c87aef..b06eadd16 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -1,7 +1,6 @@ package capacity import ( - "fmt" "github.com/bookingcom/shipper/pkg/controller/capacity/builder" "sort" "strings" @@ -11,14 +10,6 @@ import ( shipper_v1alpha1 "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" ) -type ContainerStateField string - -const ( - ContainerStateFieldType ContainerStateField = "type" - ContainerStateFieldReason ContainerStateField = "reason" - ContainerStateFieldMessage ContainerStateField = "message" -) - type containerState struct { cluster string pod string @@ -44,55 +35,6 @@ func string2ptr(v string) *string { return &v } -func getRunningContainerStateField(field ContainerStateField) string { - switch field { - case ContainerStateFieldType: - return "Running" - case ContainerStateFieldReason, ContainerStateFieldMessage: - return "" - default: - panic(fmt.Sprintf("Unknown field %s", field)) - } -} - -func getWaitingContainerStateField(stateWaiting *core_v1.ContainerStateWaiting, field ContainerStateField) string { - switch field { - case ContainerStateFieldType: - return "Waiting" - case ContainerStateFieldReason: - return stateWaiting.Reason - case ContainerStateFieldMessage: - return stateWaiting.Message - default: - panic(fmt.Sprintf("Unknown field %s", field)) - } -} - -func getTerminatedContainerStateField(stateTerminated *core_v1.ContainerStateTerminated, f ContainerStateField) string { - switch f { - case ContainerStateFieldType: - return "Terminated" - case ContainerStateFieldReason: - return stateTerminated.Reason - case ContainerStateFieldMessage: - return stateTerminated.Message - default: - panic(fmt.Sprintf("Unknown field %s", f)) - } -} - -func getContainerStateField(c core_v1.ContainerState, f ContainerStateField) string { - if c.Running != nil { - return getRunningContainerStateField(f) - } else if c.Waiting != nil { - return getWaitingContainerStateField(c.Waiting, f) - } else if c.Terminated != nil { - return getTerminatedContainerStateField(c.Terminated, f) - } - - panic("Programmer error: a container state must be either Running, Waiting or Terminated.") -} - func buildContainerStateEntries(clusterName string, podsList []*core_v1.Pod) []containerState { containerStates := make([]containerState, 0) @@ -113,9 +55,9 @@ func buildContainerStateEntries(clusterName string, podsList []*core_v1.Pod) []c state.conditionReason = string2ptr(string(cond.Reason)) for _, containerStatus := range pod.Status.ContainerStatuses { state.containerName = string2ptr(containerStatus.Name) - state.containerStateType = string2ptr(getContainerStateField(containerStatus.State, ContainerStateFieldType)) - state.containerStateReason = string2ptr(getContainerStateField(containerStatus.State, ContainerStateFieldReason)) - state.containerStateMessage = string2ptr(getContainerStateField(containerStatus.State, ContainerStateFieldMessage)) + state.containerStateType = string2ptr(builder.GetContainerStateField(containerStatus.State, builder.ContainerStateFieldType)) + state.containerStateReason = string2ptr(builder.GetContainerStateField(containerStatus.State, builder.ContainerStateFieldReason)) + state.containerStateMessage = string2ptr(builder.GetContainerStateField(containerStatus.State, builder.ContainerStateFieldMessage)) containerStates = append(containerStates, state) } From c560018c8367548c292ef6656c2e23f5a25c7081 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Thu, 14 Feb 2019 17:57:41 +0100 Subject: [PATCH 15/31] report: clean up, use builder everywhere --- .../capacity/builder/container_breakdown.go | 19 +- .../builder/pod_condition_breakdown.go | 71 ++++++- pkg/controller/capacity/builder/report.go | 64 +++++- .../capacity/capacity_controller.go | 4 +- .../capacity/capacity_controller_test.go | 143 +++++-------- pkg/controller/capacity/reporting.go | 194 +----------------- 6 files changed, 183 insertions(+), 312 deletions(-) diff --git a/pkg/controller/capacity/builder/container_breakdown.go b/pkg/controller/capacity/builder/container_breakdown.go index aa38ae3fa..90f54ae61 100644 --- a/pkg/controller/capacity/builder/container_breakdown.go +++ b/pkg/controller/capacity/builder/container_breakdown.go @@ -7,7 +7,7 @@ import ( type ContainerStateBreakdown struct { containerName string - states []v1alpha1.ClusterCapacityReportContainerStateBreakdown + states []*v1alpha1.ClusterCapacityReportContainerStateBreakdown } func NewContainerBreakdown(containerName string) *ContainerStateBreakdown { @@ -31,22 +31,29 @@ func (c *ContainerStateBreakdown) AddState( } for _, s := range c.states { - if s == breakdown { + if s.Type == breakdown.Type && s.Reason == breakdown.Reason { + s.Count += 1 return c } } - c.states = append(c.states, breakdown) + c.states = append(c.states, &breakdown) return c } func (c *ContainerStateBreakdown) Build() v1alpha1.ClusterCapacityReportContainerBreakdown { - sort.Slice(c.states, func(i, j int) bool { - return c.states[i].Type < c.states[j].Type + stateCount := len(c.states) + orderedStates := make([]v1alpha1.ClusterCapacityReportContainerStateBreakdown, stateCount) + for i, v := range c.states { + orderedStates[i] = *v + } + + sort.Slice(orderedStates, func(i, j int) bool { + return orderedStates[i].Type < orderedStates[j].Type }) return v1alpha1.ClusterCapacityReportContainerBreakdown{ Name: c.containerName, - States: c.states, + States: orderedStates, } } diff --git a/pkg/controller/capacity/builder/pod_condition_breakdown.go b/pkg/controller/capacity/builder/pod_condition_breakdown.go index 149433017..8188e6e2b 100644 --- a/pkg/controller/capacity/builder/pod_condition_breakdown.go +++ b/pkg/controller/capacity/builder/pod_condition_breakdown.go @@ -1,6 +1,21 @@ package builder -import "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" +import ( + "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" + "sort" +) + +type containerStateBreakdownBuilders map[string]*ContainerStateBreakdown + +func (c containerStateBreakdownBuilders) Get(containerName string) *ContainerStateBreakdown { + var b *ContainerStateBreakdown + var ok bool + if b, ok = c[containerName]; !ok { + b = NewContainerBreakdown(containerName) + c[containerName] = b + } + return b +} type PodConditionBreakdown struct { podCount uint32 @@ -8,35 +23,69 @@ type PodConditionBreakdown struct { podConditionStatus string name string podConditionReason string - states []v1alpha1.ClusterCapacityReportContainerStateBreakdown - containers []v1alpha1.ClusterCapacityReportContainerBreakdown + + containerStateBreakdownBuilders containerStateBreakdownBuilders } func NewPodConditionBreakdown( - podCount uint32, + initialPodCount uint32, podConditionType string, podConditionStatus string, podConditionReason string, ) *PodConditionBreakdown { return &PodConditionBreakdown{ - podCount: podCount, - podConditionType: podConditionType, - podConditionStatus: podConditionStatus, - podConditionReason: podConditionReason, + podCount: initialPodCount, + podConditionType: podConditionType, + podConditionStatus: podConditionStatus, + podConditionReason: podConditionReason, + containerStateBreakdownBuilders: make(containerStateBreakdownBuilders), } } -func (p *PodConditionBreakdown) AddContainerBreakdown(container v1alpha1.ClusterCapacityReportContainerBreakdown) *PodConditionBreakdown { - p.containers = append(p.containers, container) +func (p *PodConditionBreakdown) Key() string { + return p.podConditionType + p.podConditionStatus + p.podConditionReason +} + +func (p *PodConditionBreakdown) AddContainerStateBreakdownBuilder(containerStateBreakdown *ContainerStateBreakdown) *PodConditionBreakdown { + p.containerStateBreakdownBuilders[containerStateBreakdown.containerName] = containerStateBreakdown + return p +} + +func (p *PodConditionBreakdown) AddContainerState( + containerName string, + containerCount uint32, + podExampleName string, + containerConditionType string, + containerConditionReason string, +) *PodConditionBreakdown { + p.containerStateBreakdownBuilders. + Get(containerName). + AddState(containerCount, podExampleName, containerConditionType, containerConditionReason) + return p +} + +func (p *PodConditionBreakdown) IncrementCount() *PodConditionBreakdown { + p.podCount += 1 return p } func (p *PodConditionBreakdown) Build() v1alpha1.ClusterCapacityReportBreakdown { + + orderedContainers := make([]v1alpha1.ClusterCapacityReportContainerBreakdown, 0) + + for _, v := range p.containerStateBreakdownBuilders { + orderedContainers = append(orderedContainers, v.Build()) + } + + sort.Slice(orderedContainers, func(i, j int) bool { + return orderedContainers[i].Name < orderedContainers[i].Name + }) + return v1alpha1.ClusterCapacityReportBreakdown{ Type: p.podConditionType, Status: p.podConditionStatus, Count: p.podCount, Reason: p.podConditionReason, - Containers: p.containers, + Containers: orderedContainers, } } diff --git a/pkg/controller/capacity/builder/report.go b/pkg/controller/capacity/builder/report.go index 493d07b5a..a7f33ce9e 100644 --- a/pkg/controller/capacity/builder/report.go +++ b/pkg/controller/capacity/builder/report.go @@ -9,35 +9,81 @@ import ( "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" ) +type podConditionBreakdownBuilders map[string]*PodConditionBreakdown + +func (c podConditionBreakdownBuilders) Get(typ, status, reason string) *PodConditionBreakdown { + var b *PodConditionBreakdown + var ok bool + key := typ + status + reason + if b, ok = c[key]; !ok { + b = NewPodConditionBreakdown(0, typ, status, reason) + c[key] = b + } + return b +} + type Report struct { - ownerName string - breakdowns []v1alpha1.ClusterCapacityReportBreakdown + ownerName string + breakdowns []v1alpha1.ClusterCapacityReportBreakdown + podConditionBreakdownBuilders podConditionBreakdownBuilders } func NewReport(ownerName string) *Report { - return &Report{ownerName: ownerName} + return &Report{ + ownerName: ownerName, + podConditionBreakdownBuilders: make(podConditionBreakdownBuilders), + } } -func (r *Report) AddBreakdown(breakdown v1alpha1.ClusterCapacityReportBreakdown) *Report { - r.breakdowns = append(r.breakdowns, breakdown) - return r +func (r *Report) AddPod(pod *core_v1.Pod) { + + for _, cond := range pod.Status.Conditions { + b := r.podConditionBreakdownBuilders.Get(string(cond.Type), string(cond.Status), string(cond.Reason)) + b.IncrementCount() + + for _, containerStatus := range pod.Status.ContainerStatuses { + b.AddContainerState( + containerStatus.Name, + 1, + pod.Name, + GetContainerStateField(containerStatus.State, ContainerStateFieldType), + GetContainerStateField(containerStatus.State, ContainerStateFieldReason), + ) + } + } } func (r *Report) Build() *v1alpha1.ClusterCapacityReport { - sort.Slice(r.breakdowns, func(i, j int) bool { - return r.breakdowns[i].Type < r.breakdowns[j].Type + orderedBreakdowns := make([]v1alpha1.ClusterCapacityReportBreakdown, len(r.podConditionBreakdownBuilders)) + + i := 0 + for _, v := range r.podConditionBreakdownBuilders { + orderedBreakdowns[i] = v.Build() + i++ + } + + sort.Slice(orderedBreakdowns, func(i, j int) bool { + if orderedBreakdowns[i].Type == orderedBreakdowns[j].Type { + return orderedBreakdowns[i].Status < orderedBreakdowns[j].Status + } + return orderedBreakdowns[i].Type < orderedBreakdowns[j].Type }) report := v1alpha1.ClusterCapacityReport{ Owner: v1alpha1.ClusterCapacityReportOwner{ Name: r.ownerName, }, - Breakdown: r.breakdowns, + Breakdown: orderedBreakdowns, } return &report } +func (r *Report) AddPodConditionBreakdownBuilder(b *PodConditionBreakdown) *Report { + r.podConditionBreakdownBuilders[b.Key()] = b + return r +} + func GetRunningContainerStateField(field ContainerStateField) string { switch field { case ContainerStateFieldType: diff --git a/pkg/controller/capacity/capacity_controller.go b/pkg/controller/capacity/capacity_controller.go index e8af0f89e..577564b6a 100644 --- a/pkg/controller/capacity/capacity_controller.go +++ b/pkg/controller/capacity/capacity_controller.go @@ -354,9 +354,7 @@ func (c *Controller) getReport(targetDeployment *appsv1.Deployment, clusterStatu return nil, clusterErr } - containerStateEntries := buildContainerStateEntries(clusterStatus.Name, podsList) - conditionSummaries := summarizeContainerStatesByCondition(containerStateEntries) - report := buildReport(targetDeployment.Name, conditionSummaries) + report := buildReport(targetDeployment.Name, podsList) return report, nil } diff --git a/pkg/controller/capacity/capacity_controller_test.go b/pkg/controller/capacity/capacity_controller_test.go index 3aabc931a..c24c2f2d4 100644 --- a/pkg/controller/capacity/capacity_controller_test.go +++ b/pkg/controller/capacity/capacity_controller_test.go @@ -50,7 +50,7 @@ func TestUpdatingCapacityTargetUpdatesDeployment(t *testing.T) { }, } - f.expectCapacityTargetStatusUpdate(capacityTarget, 0, 0, expectedClusterConditions) + f.expectCapacityTargetStatusUpdate(capacityTarget, 0, 0, expectedClusterConditions, []shipper.ClusterCapacityReport{*builder.NewReport("nginx").Build()}) f.runCapacityTargetSyncHandler() } @@ -74,41 +74,24 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePod(t *testing.T f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA) c := builder.NewReport("nginx"). - AddBreakdown( - builder.NewPodConditionBreakdown(1, "ContainersReady", string(corev1.ConditionTrue), ""). - AddContainerBreakdown( - builder.NewContainerBreakdown("app"). - AddState(1, "pod-a", "Running", ""). - Build()). - Build()). - AddBreakdown( - builder.NewPodConditionBreakdown(1, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). - AddContainerBreakdown( - builder.NewContainerBreakdown("app"). - AddState(1, "pod-a", "Running", ""). - Build()). - Build()). - AddBreakdown( - builder.NewPodConditionBreakdown(1, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). - AddContainerBreakdown( - builder.NewContainerBreakdown("app"). - AddState(1, "pod-a", "Running", ""). - Build()). - Build()). - AddBreakdown( - builder.NewPodConditionBreakdown(1, "Ready", string(corev1.ConditionTrue), ""). - AddContainerBreakdown( - builder.NewContainerBreakdown("app"). - AddState(1, "pod-a", "Running", ""). - Build()). - Build()). - Build() + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(1, "ContainersReady", "True", ""). + AddContainerState("app", 1, "pod-a", "Running", "")). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(1, "Initialized", "True", ""). + AddContainerState("app", 1, "pod-a", "Running", "")). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(1, "PodScheduled", "True", ""). + AddContainerState("app", 1, "pod-a", "Running", "")). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(1, "Ready", "True", ""). + AddContainerState("app", 1, "pod-a", "Running", "")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ Name: "minikube", - Reports: []shipper.ClusterCapacityReport{*c}, + Reports: []shipper.ClusterCapacityReport{*c.Build()}, AchievedPercent: 100, AvailableReplicas: 1, Conditions: []shipper.ClusterCapacityCondition{ @@ -162,41 +145,24 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePods(t *testin f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA, podB) c := builder.NewReport("nginx"). - AddBreakdown( - builder.NewPodConditionBreakdown(2, "ContainersReady", string(corev1.ConditionTrue), ""). - AddContainerBreakdown( - builder.NewContainerBreakdown("app"). - AddState(2, "pod-a", "Running", ""). - Build()). - Build()). - AddBreakdown( - builder.NewPodConditionBreakdown(2, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). - AddContainerBreakdown( - builder.NewContainerBreakdown("app"). - AddState(2, "pod-a", "Running", ""). - Build()). - Build()). - AddBreakdown( - builder.NewPodConditionBreakdown(2, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). - AddContainerBreakdown( - builder.NewContainerBreakdown("app"). - AddState(2, "pod-a", "Running", ""). - Build()). - Build()). - AddBreakdown( - builder.NewPodConditionBreakdown(2, "Ready", string(corev1.ConditionTrue), ""). - AddContainerBreakdown( - builder.NewContainerBreakdown("app"). - AddState(2, "pod-a", "Running", ""). - Build()). - Build()). - Build() + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(2, "ContainersReady", "True", ""). + AddContainerState("app", 2, "pod-a", "Running", "")). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(2, "Initialized", "True", ""). + AddContainerState("app", 2, "pod-a", "Running", "")). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(2, "PodScheduled", "True", ""). + AddContainerState("app", 2, "pod-a", "Running", "")). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(2, "Ready", "True", ""). + AddContainerState("app", 2, "pod-a", "Running", "")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ Name: "minikube", - Reports: []shipper.ClusterCapacityReport{*c}, + Reports: []shipper.ClusterCapacityReport{*c.Build()}, AchievedPercent: 100, AvailableReplicas: 2, Conditions: []shipper.ClusterCapacityCondition{ @@ -255,31 +221,18 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA, podB, podC) c := builder.NewReport("nginx"). - AddBreakdown( + AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(3, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). - AddContainerBreakdown( - builder.NewContainerBreakdown("app"). - AddState(2, "pod-a", "Waiting", "ContainerCreating"). - AddState(1, "pod-c", "Terminated", "Completed"). - Build()). - Build()). - AddBreakdown( - builder.NewPodConditionBreakdown(3, string(corev1.PodReady), string(corev1.ConditionFalse), "ContainersNotReady"). - AddContainerBreakdown( - builder.NewContainerBreakdown("app"). - AddState(2, "pod-a", "Waiting", "ContainerCreating"). - AddState(1, "pod-c", "Terminated", "Completed"). - Build()). - Build()). - AddBreakdown( + AddContainerState("app", 2, "pod-a", "Waiting", "ContainerCreating"). + AddContainerState("app", 1, "pod-c", "Terminated", "Completed")). + AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(3, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). - AddContainerBreakdown( - builder.NewContainerBreakdown("app"). - AddState(2, "pod-a", "Waiting", "ContainerCreating"). - AddState(1, "pod-c", "Terminated", "Completed"). - Build()). - Build()). - Build() + AddContainerState("app", 2, "pod-a", "Waiting", "ContainerCreating"). + AddContainerState("app", 1, "pod-c", "Terminated", "Completed")). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(3, string(corev1.PodReady), string(corev1.ConditionFalse), "ContainersNotReady"). + AddContainerState("app", 2, "pod-a", "Waiting", "ContainerCreating"). + AddContainerState("app", 1, "pod-c", "Terminated", "Completed")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) @@ -346,7 +299,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ Name: "minikube", - Reports: []shipper.ClusterCapacityReport{*c}, + Reports: []shipper.ClusterCapacityReport{*c.Build()}, AchievedPercent: 100, AvailableReplicas: 3, Conditions: []shipper.ClusterCapacityCondition{ @@ -394,7 +347,7 @@ func TestUpdatingDeploymentsUpdatesTheCapacityTargetStatus(t *testing.T) { Message: "expected 5 replicas but have 0", }, } - f.expectCapacityTargetStatusUpdate(capacityTarget, 5, 50, clusterConditions) + f.expectCapacityTargetStatusUpdate(capacityTarget, 5, 50, clusterConditions, []shipper.ClusterCapacityReport{*builder.NewReport("nginx").Build()}) f.runCapacityTargetSyncHandler() } @@ -421,7 +374,14 @@ func TestSadPodsAreReflectedInCapacityTargetStatus(t *testing.T) { Message: "there are 1 sad pods", }, } - f.expectCapacityTargetStatusUpdate(capacityTarget, 1, 50, clusterConditions, createSadPodConditionFromPod(sadPod)) + + c := builder.NewReport("nginx"). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(1, string(corev1.PodReady), string(corev1.ConditionFalse), "ExpectedFail")). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(1, string(corev1.PodReady), string(corev1.ConditionTrue), "")) + + f.expectCapacityTargetStatusUpdate(capacityTarget, 1, 50, clusterConditions, []shipper.ClusterCapacityReport{*c.Build()}, createSadPodConditionFromPod(sadPod)) f.runCapacityTargetSyncHandler() } @@ -513,19 +473,14 @@ func (f *fixture) ExpectDeploymentPatchWithReplicas(deployment *appsv1.Deploymen f.targetClusterActions = append(f.targetClusterActions, patchAction) } -func (f *fixture) expectCapacityTargetStatusUpdate(capacityTarget *shipper.CapacityTarget, availableReplicas, achievedPercent int32, clusterConditions []shipper.ClusterCapacityCondition, sadPods ...shipper.PodStatus) { +func (f *fixture) expectCapacityTargetStatusUpdate(capacityTarget *shipper.CapacityTarget, availableReplicas, achievedPercent int32, clusterConditions []shipper.ClusterCapacityCondition, reports []shipper.ClusterCapacityReport, sadPods ...shipper.PodStatus) { clusterStatus := shipper.ClusterCapacityStatus{ Name: capacityTarget.Spec.Clusters[0].Name, AvailableReplicas: availableReplicas, AchievedPercent: achievedPercent, Conditions: clusterConditions, SadPods: sadPods, - Reports: []shipper.ClusterCapacityReport{ - { - Owner: shipper.ClusterCapacityReportOwner{Name: "nginx"}, - Breakdown: []shipper.ClusterCapacityReportBreakdown{}, - }, - }, + Reports: reports, } capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, clusterStatus) diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index b06eadd16..ef8bcd60e 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -2,207 +2,23 @@ package capacity import ( "github.com/bookingcom/shipper/pkg/controller/capacity/builder" - "sort" - "strings" - core_v1 "k8s.io/api/core/v1" + "sort" shipper_v1alpha1 "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" ) -type containerState struct { - cluster string - pod string - - conditionType *string - conditionStatus *string - conditionReason *string - - containerName *string - containerStateType *string - containerStateReason *string - containerStateMessage *string -} - -func ptr2string(v *string) string { - if v == nil { - return "" - } - return *v -} - -func string2ptr(v string) *string { - return &v -} +func buildReport(ownerName string, podsList []*core_v1.Pod) *shipper_v1alpha1.ClusterCapacityReport { -func buildContainerStateEntries(clusterName string, podsList []*core_v1.Pod) []containerState { - containerStates := make([]containerState, 0) - - // Sort pods list to offer a stable pod as example. sort.Slice(podsList, func(i, j int) bool { return podsList[i].Name < podsList[j].Name }) - for _, pod := range podsList { - state := containerState{ - cluster: clusterName, - pod: pod.Name, - } - - for _, cond := range pod.Status.Conditions { - state.conditionType = string2ptr(string(cond.Type)) - state.conditionStatus = string2ptr(string(cond.Status)) - state.conditionReason = string2ptr(string(cond.Reason)) - for _, containerStatus := range pod.Status.ContainerStatuses { - state.containerName = string2ptr(containerStatus.Name) - state.containerStateType = string2ptr(builder.GetContainerStateField(containerStatus.State, builder.ContainerStateFieldType)) - state.containerStateReason = string2ptr(builder.GetContainerStateField(containerStatus.State, builder.ContainerStateFieldReason)) - state.containerStateMessage = string2ptr(builder.GetContainerStateField(containerStatus.State, builder.ContainerStateFieldMessage)) - - containerStates = append(containerStates, state) - } - } - } - - return containerStates -} - -type containerStateSummary struct { - containerCount uint32 - example shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample - name string - reason string - typ string -} - -type conditionSummary struct { - podCount uint32 - containers map[string]containerStateSummary - reason string - status string - typ string -} - -func buildKey(args ...string) string { - validArgs := make([]string, 0) - - for _, v := range args { - if len(v) == 0 { - break - } - validArgs = append(validArgs, v) - } - - return strings.Join(validArgs, "/") -} - -func summarizeContainerStateByCondition(conditionSummaries map[string]conditionSummary, state containerState) { - - conditionSummaryKey := buildKey( - state.cluster, - ptr2string(state.conditionType), - ptr2string(state.conditionStatus), - ptr2string(state.conditionReason), - ) - - containerStateKey := buildKey( - state.cluster, - ptr2string(state.containerName), - ptr2string(state.containerStateType), - ptr2string(state.containerStateReason), - ) - - if summary, ok := conditionSummaries[conditionSummaryKey]; !ok { - - containerStates := make(map[string]containerStateSummary) - - containerStates[containerStateKey] = containerStateSummary{ - containerCount: 1, - example: shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample{Pod: state.pod}, - name: ptr2string(state.containerName), - reason: ptr2string(state.containerStateReason), - typ: ptr2string(state.containerStateType), - } - - conditionSummaries[conditionSummaryKey] = conditionSummary{ - podCount: 1, - containers: containerStates, - status: ptr2string(state.conditionStatus), - reason: ptr2string(state.conditionReason), - typ: ptr2string(state.conditionType), - } - } else { - if existingState, ok := summary.containers[containerStateKey]; !ok { - summary.containers[containerStateKey] = containerStateSummary{ - containerCount: 1, - example: shipper_v1alpha1.ClusterCapacityReportContainerBreakdownExample{Pod: state.pod}, - name: ptr2string(state.containerName), - reason: ptr2string(state.containerStateReason), - typ: ptr2string(state.containerStateType), - } - } else { - existingState.containerCount = existingState.containerCount + 1 - summary.containers[containerStateKey] = existingState - } - - summary.podCount = summary.podCount + 1 - conditionSummaries[conditionSummaryKey] = summary - } -} - -func summarizeContainerStatesByCondition(containerStates []containerState) map[string]conditionSummary { - conditionSummaries := make(map[string]conditionSummary) - for _, state := range containerStates { - summarizeContainerStateByCondition(conditionSummaries, state) - } - return conditionSummaries -} - -type conditionSummaryMap map[string]conditionSummary - -func (c conditionSummaryMap) SortedByKeyAsc() []conditionSummary { - var keys = []string{} - for k := range c { - keys = append(keys, k) - } - - sort.Slice(keys, func(i, j int) bool { - return c[keys[i]].typ < c[keys[j]].typ - }) - - var conds = []conditionSummary{} - for k := range keys { - conds = append(conds, c[keys[k]]) - } - - return conds -} - -type containerStateBreakdownBuilders map[string]*builder.ContainerStateBreakdown - -func (c containerStateBreakdownBuilders) Get(containerName string) *builder.ContainerStateBreakdown { - var b *builder.ContainerStateBreakdown - var ok bool - if b, ok = c[containerName]; !ok { - b = builder.NewContainerBreakdown(containerName) - c[containerName] = b - } - return b -} - -func buildReport(ownerName string, conditionSummaries conditionSummaryMap) *shipper_v1alpha1.ClusterCapacityReport { reportBuilder := builder.NewReport(ownerName) - containerStateBreakdownBuilders := make(containerStateBreakdownBuilders) - for _, cond := range conditionSummaries.SortedByKeyAsc() { - breakdownBuilder := builder.NewPodConditionBreakdown(cond.podCount, cond.typ, cond.status, cond.reason) - for _, container := range cond.containers { - containerStateBreakdownBuilders.Get(container.name).AddState(container.containerCount, container.example.Pod, container.typ, container.reason) - } - for _, v := range containerStateBreakdownBuilders { - breakdownBuilder.AddContainerBreakdown(v.Build()) - } - reportBuilder.AddBreakdown(breakdownBuilder.Build()) + for _, pod := range podsList { + reportBuilder.AddPod(pod) } + return reportBuilder.Build() } From e75123311e44ee95a784f105500978874570aa03 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Thu, 14 Feb 2019 18:00:41 +0100 Subject: [PATCH 16/31] report: remove unused method --- pkg/controller/capacity/builder/pod_condition_breakdown.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/controller/capacity/builder/pod_condition_breakdown.go b/pkg/controller/capacity/builder/pod_condition_breakdown.go index 8188e6e2b..373c8d259 100644 --- a/pkg/controller/capacity/builder/pod_condition_breakdown.go +++ b/pkg/controller/capacity/builder/pod_condition_breakdown.go @@ -46,11 +46,6 @@ func (p *PodConditionBreakdown) Key() string { return p.podConditionType + p.podConditionStatus + p.podConditionReason } -func (p *PodConditionBreakdown) AddContainerStateBreakdownBuilder(containerStateBreakdown *ContainerStateBreakdown) *PodConditionBreakdown { - p.containerStateBreakdownBuilders[containerStateBreakdown.containerName] = containerStateBreakdown - return p -} - func (p *PodConditionBreakdown) AddContainerState( containerName string, containerCount uint32, From 2d0781ba0a604479f119e3569c5e0085318c490a Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Thu, 14 Feb 2019 18:07:25 +0100 Subject: [PATCH 17/31] report: move breakdown building and sorting to a method --- pkg/controller/capacity/builder/report.go | 26 ++++++++++++----------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/pkg/controller/capacity/builder/report.go b/pkg/controller/capacity/builder/report.go index a7f33ce9e..92bfb1912 100644 --- a/pkg/controller/capacity/builder/report.go +++ b/pkg/controller/capacity/builder/report.go @@ -54,7 +54,20 @@ func (r *Report) AddPod(pod *core_v1.Pod) { } func (r *Report) Build() *v1alpha1.ClusterCapacityReport { + return &v1alpha1.ClusterCapacityReport{ + Owner: v1alpha1.ClusterCapacityReportOwner{ + Name: r.ownerName, + }, + Breakdown: r.buildSortedBreakdowns(), + } +} + +func (r *Report) AddPodConditionBreakdownBuilder(b *PodConditionBreakdown) *Report { + r.podConditionBreakdownBuilders[b.Key()] = b + return r +} +func (r *Report) buildSortedBreakdowns() []v1alpha1.ClusterCapacityReportBreakdown { orderedBreakdowns := make([]v1alpha1.ClusterCapacityReportBreakdown, len(r.podConditionBreakdownBuilders)) i := 0 @@ -70,18 +83,7 @@ func (r *Report) Build() *v1alpha1.ClusterCapacityReport { return orderedBreakdowns[i].Type < orderedBreakdowns[j].Type }) - report := v1alpha1.ClusterCapacityReport{ - Owner: v1alpha1.ClusterCapacityReportOwner{ - Name: r.ownerName, - }, - Breakdown: orderedBreakdowns, - } - return &report -} - -func (r *Report) AddPodConditionBreakdownBuilder(b *PodConditionBreakdown) *Report { - r.podConditionBreakdownBuilders[b.Key()] = b - return r + return orderedBreakdowns } func GetRunningContainerStateField(field ContainerStateField) string { From 6c79affe2f2fb5a83d6da5a20d607efc9e1204ff Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Thu, 14 Feb 2019 18:12:14 +0100 Subject: [PATCH 18/31] report: make linter happy * Remove unused field * Fix sort predicate implementation --- pkg/controller/capacity/builder/pod_condition_breakdown.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/controller/capacity/builder/pod_condition_breakdown.go b/pkg/controller/capacity/builder/pod_condition_breakdown.go index 373c8d259..8fab3167a 100644 --- a/pkg/controller/capacity/builder/pod_condition_breakdown.go +++ b/pkg/controller/capacity/builder/pod_condition_breakdown.go @@ -21,7 +21,6 @@ type PodConditionBreakdown struct { podCount uint32 podConditionType string podConditionStatus string - name string podConditionReason string containerStateBreakdownBuilders containerStateBreakdownBuilders @@ -73,7 +72,7 @@ func (p *PodConditionBreakdown) Build() v1alpha1.ClusterCapacityReportBreakdown } sort.Slice(orderedContainers, func(i, j int) bool { - return orderedContainers[i].Name < orderedContainers[i].Name + return orderedContainers[i].Name < orderedContainers[j].Name }) return v1alpha1.ClusterCapacityReportBreakdown{ From 034c2a54c6a2330b059c30a20f30f082545567d4 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Thu, 14 Feb 2019 18:20:12 +0100 Subject: [PATCH 19/31] report: make linter happy * Remove unused field --- pkg/controller/capacity/builder/report.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/controller/capacity/builder/report.go b/pkg/controller/capacity/builder/report.go index 92bfb1912..d33db7c4d 100644 --- a/pkg/controller/capacity/builder/report.go +++ b/pkg/controller/capacity/builder/report.go @@ -24,7 +24,6 @@ func (c podConditionBreakdownBuilders) Get(typ, status, reason string) *PodCondi type Report struct { ownerName string - breakdowns []v1alpha1.ClusterCapacityReportBreakdown podConditionBreakdownBuilders podConditionBreakdownBuilders } From a5786d1c01fbe1524680f3a1c27b017aa70fc489 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Tue, 19 Feb 2019 15:01:50 +0100 Subject: [PATCH 20/31] report: add test cases for report builder --- .../capacity/builder/report_test.go | 280 ++++++++++++++++++ 1 file changed, 280 insertions(+) create mode 100644 pkg/controller/capacity/builder/report_test.go diff --git a/pkg/controller/capacity/builder/report_test.go b/pkg/controller/capacity/builder/report_test.go new file mode 100644 index 000000000..b0ea76d5c --- /dev/null +++ b/pkg/controller/capacity/builder/report_test.go @@ -0,0 +1,280 @@ +package builder + +import ( + "github.com/pmezard/go-difflib/difflib" + "gopkg.in/yaml.v2" + "testing" + + shipper "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" +) + +func yamlDiff(a interface{}, b interface{}) (string, error) { + yamlActual, _ := yaml.Marshal(a) + yamlExpected, _ := yaml.Marshal(b) + + diff := difflib.UnifiedDiff{ + A: difflib.SplitLines(string(yamlExpected)), + B: difflib.SplitLines(string(yamlActual)), + FromFile: "Expected", + ToFile: "Actual", + Context: 4, + } + + return difflib.GetUnifiedDiffString(diff) +} + +func TestEmptyReport(t *testing.T) { + + ownerName := "owner" + + actual := NewReport(ownerName).Build() + expected := &shipper.ClusterCapacityReport{ + Owner: shipper.ClusterCapacityReportOwner{Name: ownerName}, + } + + text, err := yamlDiff(expected, actual) + if err != nil { + t.Errorf("an error occurred: %s", err) + } + if len(text) > 0 { + t.Errorf("expected is different from actual:\n%s", text) + } +} + +func TestReportOneContainerOnePodOneCondition(t *testing.T) { + ownerName := "owner" + actual := NewReport(ownerName). + AddPodConditionBreakdownBuilder( + NewPodConditionBreakdown(1, "Ready", "True", ""). + AddContainerState("app", 1, "pod-a", "Ready", "")). + Build() + expected := &shipper.ClusterCapacityReport{ + Owner: shipper.ClusterCapacityReportOwner{Name: ownerName}, + Breakdown: []shipper.ClusterCapacityReportBreakdown{ + { + Type: "Ready", + Status: "True", + Count: 1, + Containers: []shipper.ClusterCapacityReportContainerBreakdown{ + { + Name: "app", States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 1, + Type: "Ready", + Reason: "", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + }, + }, + }, + }, + }, + }, + } + + text, err := yamlDiff(expected, actual) + if err != nil { + t.Errorf("an error occurred: %s", err) + } + if len(text) > 0 { + t.Errorf("expected is different from actual:\n%s", text) + } +} + +func TestReportOneContainerTwoPodsOneCondition(t *testing.T) { + ownerName := "owner" + actual := NewReport(ownerName). + AddPodConditionBreakdownBuilder( + NewPodConditionBreakdown(2, "Ready", "True", ""). + AddContainerState("app", 1, "pod-a", "Ready", ""). + AddContainerState("app", 1, "pod-b", "Ready", "")). + Build() + + expected := &shipper.ClusterCapacityReport{ + Owner: shipper.ClusterCapacityReportOwner{Name: ownerName}, + Breakdown: []shipper.ClusterCapacityReportBreakdown{ + { + Type: "Ready", + Status: "True", + Count: 2, + Containers: []shipper.ClusterCapacityReportContainerBreakdown{ + { + Name: "app", States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 2, + Type: "Ready", + Reason: "", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + }, + }, + }, + }, + }, + }, + } + + text, err := yamlDiff(expected, actual) + if err != nil { + t.Errorf("an error occurred: %s", err) + } + if len(text) > 0 { + t.Errorf("expected is different from actual:\n%s", text) + } +} + +func TestReportTwoContainersTwoPodsOneCondition(t *testing.T) { + ownerName := "owner" + actual := NewReport(ownerName). + AddPodConditionBreakdownBuilder( + NewPodConditionBreakdown(2, "Ready", "True", ""). + AddContainerState("app", 1, "pod-a", "Ready", ""). + AddContainerState("app", 1, "pod-b", "Ready", ""). + AddContainerState("nginx", 1, "pod-a", "Ready", ""). + AddContainerState("nginx", 1, "pod-b", "Ready", "")). + Build() + + expected := &shipper.ClusterCapacityReport{ + Owner: shipper.ClusterCapacityReportOwner{Name: ownerName}, + Breakdown: []shipper.ClusterCapacityReportBreakdown{ + { + Type: "Ready", + Status: "True", + Count: 2, + Containers: []shipper.ClusterCapacityReportContainerBreakdown{ + { + Name: "app", States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 2, + Type: "Ready", + Reason: "", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + }, + }, + }, + + { + Name: "nginx", States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 2, + Type: "Ready", + Reason: "", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + }, + }, + }, + }, + }, + }, + } + + text, err := yamlDiff(expected, actual) + if err != nil { + t.Errorf("an error occurred: %s", err) + } + if len(text) > 0 { + t.Errorf("expected is different from actual:\n%s", text) + } +} + +func TestReportTwoContainersTwoPodsTwoConditions(t *testing.T) { + ownerName := "owner" + actual := NewReport(ownerName). + AddPodConditionBreakdownBuilder( + NewPodConditionBreakdown(2, "Ready", "True", ""). + AddContainerState("app", 1, "pod-a", "Ready", ""). + AddContainerState("app", 1, "pod-b", "Ready", ""). + AddContainerState("nginx", 1, "pod-a", "Ready", ""). + AddContainerState("nginx", 1, "pod-b", "Ready", "")).AddPodConditionBreakdownBuilder( + NewPodConditionBreakdown(2, "PodInitialized", "True", ""). + AddContainerState("app", 1, "pod-a", "Ready", ""). + AddContainerState("app", 1, "pod-b", "Ready", ""). + AddContainerState("nginx", 1, "pod-a", "Ready", ""). + AddContainerState("nginx", 1, "pod-b", "Ready", "")). + Build() + + expected := &shipper.ClusterCapacityReport{ + Owner: shipper.ClusterCapacityReportOwner{Name: ownerName}, + Breakdown: []shipper.ClusterCapacityReportBreakdown{ + { + Type: "PodInitialized", + Status: "True", + Count: 2, + Containers: []shipper.ClusterCapacityReportContainerBreakdown{ + { + Name: "app", States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 2, + Type: "Ready", + Reason: "", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + }, + }, + }, + + { + Name: "nginx", States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 2, + Type: "Ready", + Reason: "", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + }, + }, + }, + }, + }, + + { + Type: "Ready", + Status: "True", + Count: 2, + Containers: []shipper.ClusterCapacityReportContainerBreakdown{ + { + Name: "app", States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 2, + Type: "Ready", + Reason: "", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + }, + }, + }, + + { + Name: "nginx", States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 2, + Type: "Ready", + Reason: "", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + }, + }, + }, + }, + }, + }, + }, + } + + text, err := yamlDiff(expected, actual) + if err != nil { + t.Errorf("an error occurred: %s", err) + } + if len(text) > 0 { + t.Errorf("expected is different from actual:\n%s", text) + } +} From e242cba33470fd9a1be8e4ccc4790f2f910d0e40 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Wed, 20 Feb 2019 09:48:42 +0100 Subject: [PATCH 21/31] report: use same function to compute pod condition breakdown keys --- pkg/controller/capacity/builder/pod_condition_breakdown.go | 6 +++++- pkg/controller/capacity/builder/report.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/controller/capacity/builder/pod_condition_breakdown.go b/pkg/controller/capacity/builder/pod_condition_breakdown.go index 8fab3167a..faf0c5451 100644 --- a/pkg/controller/capacity/builder/pod_condition_breakdown.go +++ b/pkg/controller/capacity/builder/pod_condition_breakdown.go @@ -41,8 +41,12 @@ func NewPodConditionBreakdown( } } +func PodConditionBreakdownKey(typ, status, reason string) string { + return typ + status + reason +} + func (p *PodConditionBreakdown) Key() string { - return p.podConditionType + p.podConditionStatus + p.podConditionReason + return PodConditionBreakdownKey(p.podConditionType, p.podConditionStatus, p.podConditionReason) } func (p *PodConditionBreakdown) AddContainerState( diff --git a/pkg/controller/capacity/builder/report.go b/pkg/controller/capacity/builder/report.go index d33db7c4d..2379e2eab 100644 --- a/pkg/controller/capacity/builder/report.go +++ b/pkg/controller/capacity/builder/report.go @@ -14,7 +14,7 @@ type podConditionBreakdownBuilders map[string]*PodConditionBreakdown func (c podConditionBreakdownBuilders) Get(typ, status, reason string) *PodConditionBreakdown { var b *PodConditionBreakdown var ok bool - key := typ + status + reason + key := PodConditionBreakdownKey(typ, status, reason) if b, ok = c[key]; !ok { b = NewPodConditionBreakdown(0, typ, status, reason) c[key] = b From 7c4684f982a200e813a2e8bbb388780413633fd2 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Wed, 20 Feb 2019 09:52:25 +0100 Subject: [PATCH 22/31] report: chain increment count call to make it more fluid --- pkg/controller/capacity/builder/report.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/controller/capacity/builder/report.go b/pkg/controller/capacity/builder/report.go index 2379e2eab..8ba017213 100644 --- a/pkg/controller/capacity/builder/report.go +++ b/pkg/controller/capacity/builder/report.go @@ -37,8 +37,9 @@ func NewReport(ownerName string) *Report { func (r *Report) AddPod(pod *core_v1.Pod) { for _, cond := range pod.Status.Conditions { - b := r.podConditionBreakdownBuilders.Get(string(cond.Type), string(cond.Status), string(cond.Reason)) - b.IncrementCount() + b := r.podConditionBreakdownBuilders. + Get(string(cond.Type), string(cond.Status), string(cond.Reason)). + IncrementCount() for _, containerStatus := range pod.Status.ContainerStatuses { b.AddContainerState( From 5dc709fce27bdee23449187a36dc06f0357bea92 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Wed, 20 Feb 2019 09:56:51 +0100 Subject: [PATCH 23/31] report: organize import order --- pkg/controller/capacity/capacity_controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/capacity/capacity_controller_test.go b/pkg/controller/capacity/capacity_controller_test.go index c24c2f2d4..251b86d04 100644 --- a/pkg/controller/capacity/capacity_controller_test.go +++ b/pkg/controller/capacity/capacity_controller_test.go @@ -2,7 +2,6 @@ package capacity import ( "fmt" - "github.com/bookingcom/shipper/pkg/controller/capacity/builder" "sort" "testing" "time" @@ -21,6 +20,7 @@ import ( shipperfake "github.com/bookingcom/shipper/pkg/client/clientset/versioned/fake" shipperinformers "github.com/bookingcom/shipper/pkg/client/informers/externalversions" "github.com/bookingcom/shipper/pkg/conditions" + "github.com/bookingcom/shipper/pkg/controller/capacity/builder" shippertesting "github.com/bookingcom/shipper/pkg/testing" ) From 313da5793b26289ade36dcd30aded6be7bb59f57 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Wed, 20 Feb 2019 10:06:40 +0100 Subject: [PATCH 24/31] report: organize import order --- pkg/controller/capacity/builder/container_breakdown.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/controller/capacity/builder/container_breakdown.go b/pkg/controller/capacity/builder/container_breakdown.go index 90f54ae61..06e58853b 100644 --- a/pkg/controller/capacity/builder/container_breakdown.go +++ b/pkg/controller/capacity/builder/container_breakdown.go @@ -1,8 +1,9 @@ package builder import ( - "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" "sort" + + "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" ) type ContainerStateBreakdown struct { From d1a28d69d72aa1d6a654b76330d1ffed668acefc Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Wed, 20 Feb 2019 10:08:20 +0100 Subject: [PATCH 25/31] report: organize import order and add import aliases --- .../capacity/builder/container_breakdown.go | 14 +++++++------- .../capacity/builder/pod_condition_breakdown.go | 9 +++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/pkg/controller/capacity/builder/container_breakdown.go b/pkg/controller/capacity/builder/container_breakdown.go index 06e58853b..7359487a7 100644 --- a/pkg/controller/capacity/builder/container_breakdown.go +++ b/pkg/controller/capacity/builder/container_breakdown.go @@ -3,12 +3,12 @@ package builder import ( "sort" - "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" + shipper "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" ) type ContainerStateBreakdown struct { containerName string - states []*v1alpha1.ClusterCapacityReportContainerStateBreakdown + states []*shipper.ClusterCapacityReportContainerStateBreakdown } func NewContainerBreakdown(containerName string) *ContainerStateBreakdown { @@ -22,11 +22,11 @@ func (c *ContainerStateBreakdown) AddState( containerConditionReason string, ) *ContainerStateBreakdown { - breakdown := v1alpha1.ClusterCapacityReportContainerStateBreakdown{ + breakdown := shipper.ClusterCapacityReportContainerStateBreakdown{ Count: containerCount, Type: containerConditionType, Reason: containerConditionReason, - Example: v1alpha1.ClusterCapacityReportContainerBreakdownExample{ + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ Pod: podExampleName, }, } @@ -42,9 +42,9 @@ func (c *ContainerStateBreakdown) AddState( return c } -func (c *ContainerStateBreakdown) Build() v1alpha1.ClusterCapacityReportContainerBreakdown { +func (c *ContainerStateBreakdown) Build() shipper.ClusterCapacityReportContainerBreakdown { stateCount := len(c.states) - orderedStates := make([]v1alpha1.ClusterCapacityReportContainerStateBreakdown, stateCount) + orderedStates := make([]shipper.ClusterCapacityReportContainerStateBreakdown, stateCount) for i, v := range c.states { orderedStates[i] = *v } @@ -53,7 +53,7 @@ func (c *ContainerStateBreakdown) Build() v1alpha1.ClusterCapacityReportContaine return orderedStates[i].Type < orderedStates[j].Type }) - return v1alpha1.ClusterCapacityReportContainerBreakdown{ + return shipper.ClusterCapacityReportContainerBreakdown{ Name: c.containerName, States: orderedStates, } diff --git a/pkg/controller/capacity/builder/pod_condition_breakdown.go b/pkg/controller/capacity/builder/pod_condition_breakdown.go index faf0c5451..58f18d98e 100644 --- a/pkg/controller/capacity/builder/pod_condition_breakdown.go +++ b/pkg/controller/capacity/builder/pod_condition_breakdown.go @@ -1,8 +1,9 @@ package builder import ( - "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" "sort" + + shipper "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" ) type containerStateBreakdownBuilders map[string]*ContainerStateBreakdown @@ -67,9 +68,9 @@ func (p *PodConditionBreakdown) IncrementCount() *PodConditionBreakdown { return p } -func (p *PodConditionBreakdown) Build() v1alpha1.ClusterCapacityReportBreakdown { +func (p *PodConditionBreakdown) Build() shipper.ClusterCapacityReportBreakdown { - orderedContainers := make([]v1alpha1.ClusterCapacityReportContainerBreakdown, 0) + orderedContainers := make([]shipper.ClusterCapacityReportContainerBreakdown, 0) for _, v := range p.containerStateBreakdownBuilders { orderedContainers = append(orderedContainers, v.Build()) @@ -79,7 +80,7 @@ func (p *PodConditionBreakdown) Build() v1alpha1.ClusterCapacityReportBreakdown return orderedContainers[i].Name < orderedContainers[j].Name }) - return v1alpha1.ClusterCapacityReportBreakdown{ + return shipper.ClusterCapacityReportBreakdown{ Type: p.podConditionType, Status: p.podConditionStatus, Count: p.podCount, From b8903df8000de72da2590afc66aa2a77b8467799 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Wed, 20 Feb 2019 10:09:43 +0100 Subject: [PATCH 26/31] report: add import aliases --- pkg/controller/capacity/builder/report.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/controller/capacity/builder/report.go b/pkg/controller/capacity/builder/report.go index 8ba017213..e56f13e1f 100644 --- a/pkg/controller/capacity/builder/report.go +++ b/pkg/controller/capacity/builder/report.go @@ -6,7 +6,7 @@ import ( core_v1 "k8s.io/api/core/v1" - "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" + shipper "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" ) type podConditionBreakdownBuilders map[string]*PodConditionBreakdown @@ -53,9 +53,9 @@ func (r *Report) AddPod(pod *core_v1.Pod) { } } -func (r *Report) Build() *v1alpha1.ClusterCapacityReport { - return &v1alpha1.ClusterCapacityReport{ - Owner: v1alpha1.ClusterCapacityReportOwner{ +func (r *Report) Build() *shipper.ClusterCapacityReport { + return &shipper.ClusterCapacityReport{ + Owner: shipper.ClusterCapacityReportOwner{ Name: r.ownerName, }, Breakdown: r.buildSortedBreakdowns(), @@ -67,8 +67,8 @@ func (r *Report) AddPodConditionBreakdownBuilder(b *PodConditionBreakdown) *Repo return r } -func (r *Report) buildSortedBreakdowns() []v1alpha1.ClusterCapacityReportBreakdown { - orderedBreakdowns := make([]v1alpha1.ClusterCapacityReportBreakdown, len(r.podConditionBreakdownBuilders)) +func (r *Report) buildSortedBreakdowns() []shipper.ClusterCapacityReportBreakdown { + orderedBreakdowns := make([]shipper.ClusterCapacityReportBreakdown, len(r.podConditionBreakdownBuilders)) i := 0 for _, v := range r.podConditionBreakdownBuilders { From 717abb556e0e3d1f6f76a73292653666e90354ef Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Wed, 20 Feb 2019 10:11:44 +0100 Subject: [PATCH 27/31] report: organize import order and add import aliases --- pkg/controller/capacity/reporting.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/controller/capacity/reporting.go b/pkg/controller/capacity/reporting.go index ef8bcd60e..7c0796d9c 100644 --- a/pkg/controller/capacity/reporting.go +++ b/pkg/controller/capacity/reporting.go @@ -1,14 +1,15 @@ package capacity import ( - "github.com/bookingcom/shipper/pkg/controller/capacity/builder" - core_v1 "k8s.io/api/core/v1" "sort" - shipper_v1alpha1 "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" + core_v1 "k8s.io/api/core/v1" + + shipper "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" + "github.com/bookingcom/shipper/pkg/controller/capacity/builder" ) -func buildReport(ownerName string, podsList []*core_v1.Pod) *shipper_v1alpha1.ClusterCapacityReport { +func buildReport(ownerName string, podsList []*core_v1.Pod) *shipper.ClusterCapacityReport { sort.Slice(podsList, func(i, j int) bool { return podsList[i].Name < podsList[j].Name From e8f179bde09800afe20c4f08712c9577ec5c2c0f Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Wed, 20 Feb 2019 15:10:29 +0100 Subject: [PATCH 28/31] report: add container breakdown example message --- pkg/apis/shipper/v1alpha1/types.go | 3 +- .../capacity/builder/container_breakdown.go | 9 +- .../builder/pod_condition_breakdown.go | 3 +- pkg/controller/capacity/builder/report.go | 22 ++ .../capacity/builder/report_test.go | 76 ++++-- .../capacity/capacity_controller_test.go | 244 ++++++++++++++++-- pkg/controller/capacity/utils_test.go | 19 +- 7 files changed, 331 insertions(+), 45 deletions(-) diff --git a/pkg/apis/shipper/v1alpha1/types.go b/pkg/apis/shipper/v1alpha1/types.go index 03fe2b7cc..bed8c3d33 100644 --- a/pkg/apis/shipper/v1alpha1/types.go +++ b/pkg/apis/shipper/v1alpha1/types.go @@ -337,7 +337,8 @@ type CapacityTargetStatus struct { } type ClusterCapacityReportContainerBreakdownExample struct { - Pod string `json:"pod"` + Pod string `json:"pod"` + Message *string `json:"message,omitempty"` } type ClusterCapacityReportContainerStateBreakdown struct { diff --git a/pkg/controller/capacity/builder/container_breakdown.go b/pkg/controller/capacity/builder/container_breakdown.go index 7359487a7..aecf59f36 100644 --- a/pkg/controller/capacity/builder/container_breakdown.go +++ b/pkg/controller/capacity/builder/container_breakdown.go @@ -20,14 +20,21 @@ func (c *ContainerStateBreakdown) AddState( podExampleName string, containerConditionType string, containerConditionReason string, + containerExampleMessage string, ) *ContainerStateBreakdown { + var m *string + if len(containerExampleMessage) > 0 { + m = &containerExampleMessage + } + breakdown := shipper.ClusterCapacityReportContainerStateBreakdown{ Count: containerCount, Type: containerConditionType, Reason: containerConditionReason, Example: shipper.ClusterCapacityReportContainerBreakdownExample{ - Pod: podExampleName, + Pod: podExampleName, + Message: m, }, } diff --git a/pkg/controller/capacity/builder/pod_condition_breakdown.go b/pkg/controller/capacity/builder/pod_condition_breakdown.go index 58f18d98e..f7b0e43f5 100644 --- a/pkg/controller/capacity/builder/pod_condition_breakdown.go +++ b/pkg/controller/capacity/builder/pod_condition_breakdown.go @@ -56,10 +56,11 @@ func (p *PodConditionBreakdown) AddContainerState( podExampleName string, containerConditionType string, containerConditionReason string, + containerExampleMessage string, ) *PodConditionBreakdown { p.containerStateBreakdownBuilders. Get(containerName). - AddState(containerCount, podExampleName, containerConditionType, containerConditionReason) + AddState(containerCount, podExampleName, containerConditionType, containerConditionReason, containerExampleMessage) return p } diff --git a/pkg/controller/capacity/builder/report.go b/pkg/controller/capacity/builder/report.go index e56f13e1f..35875ea64 100644 --- a/pkg/controller/capacity/builder/report.go +++ b/pkg/controller/capacity/builder/report.go @@ -48,6 +48,7 @@ func (r *Report) AddPod(pod *core_v1.Pod) { pod.Name, GetContainerStateField(containerStatus.State, ContainerStateFieldType), GetContainerStateField(containerStatus.State, ContainerStateFieldReason), + GetContainerStateMessage(containerStatus), ) } } @@ -135,6 +136,27 @@ func GetContainerStateField(c core_v1.ContainerState, f ContainerStateField) str panic("Programmer error: a container state must be either Running, Waiting or Terminated.") } +func getTerminatedMessage(c core_v1.ContainerState) string { + if c.Terminated == nil { + return "" + } + + if len(c.Terminated.Message) > 0 { + return c.Terminated.Message + } else if c.Terminated.Signal > 0 { + return fmt.Sprintf("Terminated with signal %d", c.Terminated.Signal) + } else { + return fmt.Sprintf("Terminated with exit code %d", c.Terminated.ExitCode) + } +} + +func GetContainerStateMessage(c core_v1.ContainerStatus) string { + if c.RestartCount > 0 { + return getTerminatedMessage(c.LastTerminationState) + } + return getTerminatedMessage(c.State) +} + type ContainerStateField string const ( diff --git a/pkg/controller/capacity/builder/report_test.go b/pkg/controller/capacity/builder/report_test.go index b0ea76d5c..f5f1cd9d9 100644 --- a/pkg/controller/capacity/builder/report_test.go +++ b/pkg/controller/capacity/builder/report_test.go @@ -46,7 +46,7 @@ func TestReportOneContainerOnePodOneCondition(t *testing.T) { actual := NewReport(ownerName). AddPodConditionBreakdownBuilder( NewPodConditionBreakdown(1, "Ready", "True", ""). - AddContainerState("app", 1, "pod-a", "Ready", "")). + AddContainerState("app", 1, "pod-a", "Ready", "", "")). Build() expected := &shipper.ClusterCapacityReport{ Owner: shipper.ClusterCapacityReportOwner{Name: ownerName}, @@ -82,13 +82,59 @@ func TestReportOneContainerOnePodOneCondition(t *testing.T) { } } +func TestReportOneContainerOnePodOneConditionTerminatedWithExitCodeContainer(t *testing.T) { + ownerName := "owner" + actual := NewReport(ownerName). + AddPodConditionBreakdownBuilder( + NewPodConditionBreakdown(1, "Ready", "True", ""). + AddContainerState("app", 1, "pod-a", "Ready", "", "Terminated with exit code 1")). + Build() + + m := "Terminated with exit code 1" + mPtr := &m + + expected := &shipper.ClusterCapacityReport{ + Owner: shipper.ClusterCapacityReportOwner{Name: ownerName}, + Breakdown: []shipper.ClusterCapacityReportBreakdown{ + { + Type: "Ready", + Status: "True", + Count: 1, + Containers: []shipper.ClusterCapacityReportContainerBreakdown{ + { + Name: "app", States: []shipper.ClusterCapacityReportContainerStateBreakdown{ + { + Count: 1, + Type: "Ready", + Reason: "", + Example: shipper.ClusterCapacityReportContainerBreakdownExample{ + Pod: "pod-a", + Message: mPtr, + }, + }, + }, + }, + }, + }, + }, + } + + text, err := yamlDiff(expected, actual) + if err != nil { + t.Errorf("an error occurred: %s", err) + } + if len(text) > 0 { + t.Errorf("expected is different from actual:\n%s", text) + } +} + func TestReportOneContainerTwoPodsOneCondition(t *testing.T) { ownerName := "owner" actual := NewReport(ownerName). AddPodConditionBreakdownBuilder( NewPodConditionBreakdown(2, "Ready", "True", ""). - AddContainerState("app", 1, "pod-a", "Ready", ""). - AddContainerState("app", 1, "pod-b", "Ready", "")). + AddContainerState("app", 1, "pod-a", "Ready", "", ""). + AddContainerState("app", 1, "pod-b", "Ready", "", "")). Build() expected := &shipper.ClusterCapacityReport{ @@ -130,10 +176,10 @@ func TestReportTwoContainersTwoPodsOneCondition(t *testing.T) { actual := NewReport(ownerName). AddPodConditionBreakdownBuilder( NewPodConditionBreakdown(2, "Ready", "True", ""). - AddContainerState("app", 1, "pod-a", "Ready", ""). - AddContainerState("app", 1, "pod-b", "Ready", ""). - AddContainerState("nginx", 1, "pod-a", "Ready", ""). - AddContainerState("nginx", 1, "pod-b", "Ready", "")). + AddContainerState("app", 1, "pod-a", "Ready", "", ""). + AddContainerState("app", 1, "pod-b", "Ready", "", ""). + AddContainerState("nginx", 1, "pod-a", "Ready", "", ""). + AddContainerState("nginx", 1, "pod-b", "Ready", "", "")). Build() expected := &shipper.ClusterCapacityReport{ @@ -188,15 +234,15 @@ func TestReportTwoContainersTwoPodsTwoConditions(t *testing.T) { actual := NewReport(ownerName). AddPodConditionBreakdownBuilder( NewPodConditionBreakdown(2, "Ready", "True", ""). - AddContainerState("app", 1, "pod-a", "Ready", ""). - AddContainerState("app", 1, "pod-b", "Ready", ""). - AddContainerState("nginx", 1, "pod-a", "Ready", ""). - AddContainerState("nginx", 1, "pod-b", "Ready", "")).AddPodConditionBreakdownBuilder( + AddContainerState("app", 1, "pod-a", "Ready", "", ""). + AddContainerState("app", 1, "pod-b", "Ready", "", ""). + AddContainerState("nginx", 1, "pod-a", "Ready", "", ""). + AddContainerState("nginx", 1, "pod-b", "Ready", "", "")).AddPodConditionBreakdownBuilder( NewPodConditionBreakdown(2, "PodInitialized", "True", ""). - AddContainerState("app", 1, "pod-a", "Ready", ""). - AddContainerState("app", 1, "pod-b", "Ready", ""). - AddContainerState("nginx", 1, "pod-a", "Ready", ""). - AddContainerState("nginx", 1, "pod-b", "Ready", "")). + AddContainerState("app", 1, "pod-a", "Ready", "", ""). + AddContainerState("app", 1, "pod-b", "Ready", "", ""). + AddContainerState("nginx", 1, "pod-a", "Ready", "", ""). + AddContainerState("nginx", 1, "pod-b", "Ready", "", "")). Build() expected := &shipper.ClusterCapacityReport{ diff --git a/pkg/controller/capacity/capacity_controller_test.go b/pkg/controller/capacity/capacity_controller_test.go index 251b86d04..d466f27a3 100644 --- a/pkg/controller/capacity/capacity_controller_test.go +++ b/pkg/controller/capacity/capacity_controller_test.go @@ -64,7 +64,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePod(t *testing.T podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) podA := newPodBuilder("pod-a", deployment.GetNamespace(), podLabels). - AddContainerStatus("app", corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}). + AddContainerStatus("app", corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, 0, nil). AddPodCondition(corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionTrue}). AddPodCondition(corev1.PodCondition{Type: "ContainersReady", Status: corev1.ConditionTrue}). AddPodCondition(corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionTrue}). @@ -76,16 +76,220 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePod(t *testing.T c := builder.NewReport("nginx"). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(1, "ContainersReady", "True", ""). - AddContainerState("app", 1, "pod-a", "Running", "")). + AddContainerState("app", 1, "pod-a", "Running", "", "")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(1, "Initialized", "True", ""). - AddContainerState("app", 1, "pod-a", "Running", "")). + AddContainerState("app", 1, "pod-a", "Running", "", "")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(1, "PodScheduled", "True", ""). - AddContainerState("app", 1, "pod-a", "Running", "")). + AddContainerState("app", 1, "pod-a", "Running", "", "")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(1, "Ready", "True", ""). - AddContainerState("app", 1, "pod-a", "Running", "")) + AddContainerState("app", 1, "pod-a", "Running", "", "")) + + f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) + + capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ + Name: "minikube", + Reports: []shipper.ClusterCapacityReport{*c.Build()}, + AchievedPercent: 100, + AvailableReplicas: 1, + Conditions: []shipper.ClusterCapacityCondition{ + {Type: shipper.ClusterConditionTypeOperational, Status: corev1.ConditionTrue}, + {Type: shipper.ClusterConditionTypeReady, Status: corev1.ConditionTrue}, + }, + }) + + updateAction := kubetesting.NewUpdateAction( + schema.GroupVersionResource{ + Group: shipper.SchemeGroupVersion.Group, + Version: shipper.SchemeGroupVersion.Version, + Resource: "capacitytargets", + }, + capacityTarget.GetNamespace(), + capacityTarget, + ) + + f.managementClusterActions = append(f.managementClusterActions, updateAction) + f.runCapacityTargetSyncHandler() + + // Calling the sync handler again with the updated capacity target object should yield the same results. + f.managementObjects = []runtime.Object{capacityTarget.DeepCopy()} + f.runCapacityTargetSyncHandler() +} + +func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePodCompletedContainer(t *testing.T) { + f := NewFixture(t) + + capacityTarget := newCapacityTarget(1, 100) + + deployment := newDeployment(1, 1) + podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) + + podA := newPodBuilder("pod-a", deployment.GetNamespace(), podLabels). + AddContainerStatus("app", corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "Completed", ExitCode: 1}}, 0, nil). + AddPodCondition(corev1.PodCondition{Type: "ContainersReady", Status: corev1.ConditionTrue}). + Build() + + f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA) + + c := builder.NewReport("nginx"). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(1, "ContainersReady", "True", ""). + AddContainerState("app", 1, "pod-a", "Terminated", "Completed", "Terminated with exit code 1")) + + f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) + + capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ + Name: "minikube", + Reports: []shipper.ClusterCapacityReport{*c.Build()}, + AchievedPercent: 100, + AvailableReplicas: 1, + Conditions: []shipper.ClusterCapacityCondition{ + {Type: shipper.ClusterConditionTypeOperational, Status: corev1.ConditionTrue}, + {Type: shipper.ClusterConditionTypeReady, Status: corev1.ConditionTrue}, + }, + }) + + updateAction := kubetesting.NewUpdateAction( + schema.GroupVersionResource{ + Group: shipper.SchemeGroupVersion.Group, + Version: shipper.SchemeGroupVersion.Version, + Resource: "capacitytargets", + }, + capacityTarget.GetNamespace(), + capacityTarget, + ) + + f.managementClusterActions = append(f.managementClusterActions, updateAction) + f.runCapacityTargetSyncHandler() + + // Calling the sync handler again with the updated capacity target object should yield the same results. + f.managementObjects = []runtime.Object{capacityTarget.DeepCopy()} + f.runCapacityTargetSyncHandler() +} + +func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePodTerminatedContainer(t *testing.T) { + f := NewFixture(t) + + capacityTarget := newCapacityTarget(1, 100) + + deployment := newDeployment(1, 1) + podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) + + podA := newPodBuilder("pod-a", deployment.GetNamespace(), podLabels). + AddContainerStatus("app", corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "Terminated", Signal: 9}}, 0, nil). + AddPodCondition(corev1.PodCondition{Type: "ContainersReady", Status: corev1.ConditionTrue}). + Build() + + f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA) + + c := builder.NewReport("nginx"). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(1, "ContainersReady", "True", ""). + AddContainerState("app", 1, "pod-a", "Terminated", "Terminated", "Terminated with signal 9")) + + f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) + + capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ + Name: "minikube", + Reports: []shipper.ClusterCapacityReport{*c.Build()}, + AchievedPercent: 100, + AvailableReplicas: 1, + Conditions: []shipper.ClusterCapacityCondition{ + {Type: shipper.ClusterConditionTypeOperational, Status: corev1.ConditionTrue}, + {Type: shipper.ClusterConditionTypeReady, Status: corev1.ConditionTrue}, + }, + }) + + updateAction := kubetesting.NewUpdateAction( + schema.GroupVersionResource{ + Group: shipper.SchemeGroupVersion.Group, + Version: shipper.SchemeGroupVersion.Version, + Resource: "capacitytargets", + }, + capacityTarget.GetNamespace(), + capacityTarget, + ) + + f.managementClusterActions = append(f.managementClusterActions, updateAction) + f.runCapacityTargetSyncHandler() + + // Calling the sync handler again with the updated capacity target object should yield the same results. + f.managementObjects = []runtime.Object{capacityTarget.DeepCopy()} + f.runCapacityTargetSyncHandler() +} + +func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePodRestartedContainer(t *testing.T) { + f := NewFixture(t) + + capacityTarget := newCapacityTarget(1, 100) + + deployment := newDeployment(1, 1) + podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) + + podA := newPodBuilder("pod-a", deployment.GetNamespace(), podLabels). + AddContainerStatus("app", corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "Terminated", Signal: 9}}, 0, nil). + AddPodCondition(corev1.PodCondition{Type: "ContainersReady", Status: corev1.ConditionTrue}). + Build() + + f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA) + + c := builder.NewReport("nginx"). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(1, "ContainersReady", "True", ""). + AddContainerState("app", 1, "pod-a", "Terminated", "Terminated", "Terminated with signal 9")) + + f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) + + capacityTarget.Status.Clusters = append(capacityTarget.Status.Clusters, shipper.ClusterCapacityStatus{ + Name: "minikube", + Reports: []shipper.ClusterCapacityReport{*c.Build()}, + AchievedPercent: 100, + AvailableReplicas: 1, + Conditions: []shipper.ClusterCapacityCondition{ + {Type: shipper.ClusterConditionTypeOperational, Status: corev1.ConditionTrue}, + {Type: shipper.ClusterConditionTypeReady, Status: corev1.ConditionTrue}, + }, + }) + + updateAction := kubetesting.NewUpdateAction( + schema.GroupVersionResource{ + Group: shipper.SchemeGroupVersion.Group, + Version: shipper.SchemeGroupVersion.Version, + Resource: "capacitytargets", + }, + capacityTarget.GetNamespace(), + capacityTarget, + ) + + f.managementClusterActions = append(f.managementClusterActions, updateAction) + f.runCapacityTargetSyncHandler() + + // Calling the sync handler again with the updated capacity target object should yield the same results. + f.managementObjects = []runtime.Object{capacityTarget.DeepCopy()} + f.runCapacityTargetSyncHandler() +} + +func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePodRestartedContainerWithTerminationMessage(t *testing.T) { + f := NewFixture(t) + + capacityTarget := newCapacityTarget(1, 100) + + deployment := newDeployment(1, 1) + podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) + + podA := newPodBuilder("pod-a", deployment.GetNamespace(), podLabels). + AddContainerStatus("app", corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "Terminated", Signal: 9}}, 1, &corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Message: "termination message"}}). + AddPodCondition(corev1.PodCondition{Type: "ContainersReady", Status: corev1.ConditionTrue}). + Build() + + f.targetClusterObjects = append(f.targetClusterObjects, deployment, podA) + + c := builder.NewReport("nginx"). + AddPodConditionBreakdownBuilder( + builder.NewPodConditionBreakdown(1, "ContainersReady", "True", ""). + AddContainerState("app", 1, "pod-a", "Terminated", "Terminated", "termination message")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) @@ -127,7 +331,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePods(t *testin podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) podA := newPodBuilder("pod-a", deployment.GetNamespace(), podLabels). - AddContainerStatus("app", corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}). + AddContainerStatus("app", corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, 0, nil). AddPodCondition(corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionTrue}). AddPodCondition(corev1.PodCondition{Type: "ContainersReady", Status: corev1.ConditionTrue}). AddPodCondition(corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionTrue}). @@ -135,7 +339,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePods(t *testin Build() podB := newPodBuilder("pod-b", deployment.GetNamespace(), podLabels). - AddContainerStatus("app", corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}). + AddContainerStatus("app", corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, 0, nil). AddPodCondition(corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionTrue}). AddPodCondition(corev1.PodCondition{Type: "ContainersReady", Status: corev1.ConditionTrue}). AddPodCondition(corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionTrue}). @@ -147,16 +351,16 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePods(t *testin c := builder.NewReport("nginx"). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(2, "ContainersReady", "True", ""). - AddContainerState("app", 2, "pod-a", "Running", "")). + AddContainerState("app", 2, "pod-a", "Running", "", "")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(2, "Initialized", "True", ""). - AddContainerState("app", 2, "pod-a", "Running", "")). + AddContainerState("app", 2, "pod-a", "Running", "", "")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(2, "PodScheduled", "True", ""). - AddContainerState("app", 2, "pod-a", "Running", "")). + AddContainerState("app", 2, "pod-a", "Running", "", "")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(2, "Ready", "True", ""). - AddContainerState("app", 2, "pod-a", "Running", "")) + AddContainerState("app", 2, "pod-a", "Running", "", "")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) @@ -198,21 +402,21 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer podLabels, _ := metav1.LabelSelectorAsMap(deployment.Spec.Selector) podA := newPodBuilder("pod-a", deployment.GetNamespace(), podLabels). - AddContainerStatus("app", corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ContainerCreating"}}). + AddContainerStatus("app", corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ContainerCreating"}}, 0, nil). AddPodCondition(corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionTrue}). AddPodCondition(corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionFalse, Reason: "ContainersNotReady"}). AddPodCondition(corev1.PodCondition{Type: corev1.PodScheduled, Status: corev1.ConditionTrue}). Build() podB := newPodBuilder("pod-b", deployment.GetNamespace(), podLabels). - AddContainerStatus("app", corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ContainerCreating"}}). + AddContainerStatus("app", corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ContainerCreating"}}, 0, nil). AddPodCondition(corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionTrue}). AddPodCondition(corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionFalse, Reason: "ContainersNotReady"}). AddPodCondition(corev1.PodCondition{Type: corev1.PodScheduled, Status: corev1.ConditionTrue}). Build() podC := newPodBuilder("pod-c", deployment.GetNamespace(), podLabels). - AddContainerStatus("app", corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "Completed"}}). + AddContainerStatus("app", corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "Completed"}}, 0, nil). AddPodCondition(corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionTrue}). AddPodCondition(corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionFalse, Reason: "ContainersNotReady"}). AddPodCondition(corev1.PodCondition{Type: corev1.PodScheduled, Status: corev1.ConditionTrue}). @@ -223,16 +427,16 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer c := builder.NewReport("nginx"). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(3, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). - AddContainerState("app", 2, "pod-a", "Waiting", "ContainerCreating"). - AddContainerState("app", 1, "pod-c", "Terminated", "Completed")). + AddContainerState("app", 2, "pod-a", "Waiting", "ContainerCreating", ""). + AddContainerState("app", 1, "pod-c", "Terminated", "Completed", "Terminated with exit code 0")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(3, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). - AddContainerState("app", 2, "pod-a", "Waiting", "ContainerCreating"). - AddContainerState("app", 1, "pod-c", "Terminated", "Completed")). + AddContainerState("app", 2, "pod-a", "Waiting", "ContainerCreating", ""). + AddContainerState("app", 1, "pod-c", "Terminated", "Completed", "Terminated with exit code 0")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(3, string(corev1.PodReady), string(corev1.ConditionFalse), "ContainersNotReady"). - AddContainerState("app", 2, "pod-a", "Waiting", "ContainerCreating"). - AddContainerState("app", 1, "pod-c", "Terminated", "Completed")) + AddContainerState("app", 2, "pod-a", "Waiting", "ContainerCreating", ""). + AddContainerState("app", 1, "pod-c", "Terminated", "Completed", "Terminated with exit code 0")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) diff --git a/pkg/controller/capacity/utils_test.go b/pkg/controller/capacity/utils_test.go index b374c2813..249e29e96 100644 --- a/pkg/controller/capacity/utils_test.go +++ b/pkg/controller/capacity/utils_test.go @@ -8,11 +8,12 @@ import ( ) type PodBuilder struct { - podName string - podNamespace string - podLabels map[string]string - containerStatuses []corev1.ContainerStatus - podConditions []corev1.PodCondition + podName string + podNamespace string + podLabels map[string]string + containerStatuses []corev1.ContainerStatus + podConditions []corev1.PodCondition + lastTerminationState corev1.ContainerState } func newPodBuilder(podName string, podNamespace string, podLabels map[string]string) *PodBuilder { @@ -38,8 +39,12 @@ func (p *PodBuilder) SetLabels(labels map[string]string) *PodBuilder { return p } -func (p *PodBuilder) AddContainerStatus(containerName string, containerState corev1.ContainerState) *PodBuilder { - p.containerStatuses = append(p.containerStatuses, corev1.ContainerStatus{Name: containerName, State: containerState}) +func (p *PodBuilder) AddContainerStatus(containerName string, containerState corev1.ContainerState, restartCount int32, lastTerminatedState *corev1.ContainerState) *PodBuilder { + containerStatus := corev1.ContainerStatus{Name: containerName, State: containerState, RestartCount: restartCount} + if lastTerminatedState != nil { + containerStatus.LastTerminationState = *lastTerminatedState + } + p.containerStatuses = append(p.containerStatuses, containerStatus) return p } From 7d258b09f38dd6f6dce5681b34f79902031f1f4e Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Fri, 22 Feb 2019 13:49:16 +0100 Subject: [PATCH 29/31] hack/update-codegen.sh --- .../shipper/v1alpha1/zz_generated.deepcopy.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/apis/shipper/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/shipper/v1alpha1/zz_generated.deepcopy.go index 172e71bc0..6d916d350 100644 --- a/pkg/apis/shipper/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/shipper/v1alpha1/zz_generated.deepcopy.go @@ -397,7 +397,9 @@ func (in *ClusterCapacityReportContainerBreakdown) DeepCopyInto(out *ClusterCapa if in.States != nil { in, out := &in.States, &out.States *out = make([]ClusterCapacityReportContainerStateBreakdown, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -415,6 +417,15 @@ func (in *ClusterCapacityReportContainerBreakdown) DeepCopy() *ClusterCapacityRe // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterCapacityReportContainerBreakdownExample) DeepCopyInto(out *ClusterCapacityReportContainerBreakdownExample) { *out = *in + if in.Message != nil { + in, out := &in.Message, &out.Message + if *in == nil { + *out = nil + } else { + *out = new(string) + **out = **in + } + } return } @@ -431,7 +442,7 @@ func (in *ClusterCapacityReportContainerBreakdownExample) DeepCopy() *ClusterCap // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterCapacityReportContainerStateBreakdown) DeepCopyInto(out *ClusterCapacityReportContainerStateBreakdown) { *out = *in - out.Example = in.Example + in.Example.DeepCopyInto(&out.Example) return } From a71ccdfe01d6f96da3c8d69108c1cd642d6efb58 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Fri, 22 Feb 2019 15:36:14 +0100 Subject: [PATCH 30/31] reporting: remove unused field --- pkg/controller/capacity/utils_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/controller/capacity/utils_test.go b/pkg/controller/capacity/utils_test.go index 249e29e96..50468ad4f 100644 --- a/pkg/controller/capacity/utils_test.go +++ b/pkg/controller/capacity/utils_test.go @@ -8,12 +8,11 @@ import ( ) type PodBuilder struct { - podName string - podNamespace string - podLabels map[string]string - containerStatuses []corev1.ContainerStatus - podConditions []corev1.PodCondition - lastTerminationState corev1.ContainerState + podName string + podNamespace string + podLabels map[string]string + containerStatuses []corev1.ContainerStatus + podConditions []corev1.PodCondition } func newPodBuilder(podName string, podNamespace string, podLabels map[string]string) *PodBuilder { From 3af17cbc64ccc0a82542e3930aef76be3648f559 Mon Sep 17 00:00:00 2001 From: Parham Doustdar Date: Mon, 11 Mar 2019 15:17:31 +0100 Subject: [PATCH 31/31] Removed the `initialCount` from `AddContainerState`, and renamed the function to `AddOrIncrementContainerState` since it matches what the function does more closely. Also changed `AddState` to `AddOrIncrementState` and removed the argument there, too. I'm leaving the `initialCount` argument of `NewPodBuilder` untouched because that one is actually set at object instantiation time, and it's always taken into account, unlike the two functions I mentioned above. --- .../capacity/builder/container_breakdown.go | 20 ++++----- .../builder/pod_condition_breakdown.go | 5 +-- pkg/controller/capacity/builder/report.go | 4 +- .../capacity/builder/report_test.go | 35 +++++++-------- .../capacity/capacity_controller_test.go | 43 +++++++++++-------- 5 files changed, 55 insertions(+), 52 deletions(-) diff --git a/pkg/controller/capacity/builder/container_breakdown.go b/pkg/controller/capacity/builder/container_breakdown.go index aecf59f36..1c0a52853 100644 --- a/pkg/controller/capacity/builder/container_breakdown.go +++ b/pkg/controller/capacity/builder/container_breakdown.go @@ -15,8 +15,7 @@ func NewContainerBreakdown(containerName string) *ContainerStateBreakdown { return &ContainerStateBreakdown{containerName: containerName} } -func (c *ContainerStateBreakdown) AddState( - containerCount uint32, +func (c *ContainerStateBreakdown) AddOrIncrementState( podExampleName string, containerConditionType string, containerConditionReason string, @@ -28,8 +27,15 @@ func (c *ContainerStateBreakdown) AddState( m = &containerExampleMessage } + for _, s := range c.states { + if s.Type == containerConditionType && s.Reason == containerConditionReason { + s.Count += 1 + return c + } + } + breakdown := shipper.ClusterCapacityReportContainerStateBreakdown{ - Count: containerCount, + Count: 1, Type: containerConditionType, Reason: containerConditionReason, Example: shipper.ClusterCapacityReportContainerBreakdownExample{ @@ -37,14 +43,6 @@ func (c *ContainerStateBreakdown) AddState( Message: m, }, } - - for _, s := range c.states { - if s.Type == breakdown.Type && s.Reason == breakdown.Reason { - s.Count += 1 - return c - } - } - c.states = append(c.states, &breakdown) return c } diff --git a/pkg/controller/capacity/builder/pod_condition_breakdown.go b/pkg/controller/capacity/builder/pod_condition_breakdown.go index f7b0e43f5..117645561 100644 --- a/pkg/controller/capacity/builder/pod_condition_breakdown.go +++ b/pkg/controller/capacity/builder/pod_condition_breakdown.go @@ -50,9 +50,8 @@ func (p *PodConditionBreakdown) Key() string { return PodConditionBreakdownKey(p.podConditionType, p.podConditionStatus, p.podConditionReason) } -func (p *PodConditionBreakdown) AddContainerState( +func (p *PodConditionBreakdown) AddOrIncrementContainerState( containerName string, - containerCount uint32, podExampleName string, containerConditionType string, containerConditionReason string, @@ -60,7 +59,7 @@ func (p *PodConditionBreakdown) AddContainerState( ) *PodConditionBreakdown { p.containerStateBreakdownBuilders. Get(containerName). - AddState(containerCount, podExampleName, containerConditionType, containerConditionReason, containerExampleMessage) + AddOrIncrementState(podExampleName, containerConditionType, containerConditionReason, containerExampleMessage) return p } diff --git a/pkg/controller/capacity/builder/report.go b/pkg/controller/capacity/builder/report.go index 35875ea64..2752b1d71 100644 --- a/pkg/controller/capacity/builder/report.go +++ b/pkg/controller/capacity/builder/report.go @@ -35,16 +35,14 @@ func NewReport(ownerName string) *Report { } func (r *Report) AddPod(pod *core_v1.Pod) { - for _, cond := range pod.Status.Conditions { b := r.podConditionBreakdownBuilders. Get(string(cond.Type), string(cond.Status), string(cond.Reason)). IncrementCount() for _, containerStatus := range pod.Status.ContainerStatuses { - b.AddContainerState( + b.AddOrIncrementContainerState( containerStatus.Name, - 1, pod.Name, GetContainerStateField(containerStatus.State, ContainerStateFieldType), GetContainerStateField(containerStatus.State, ContainerStateFieldReason), diff --git a/pkg/controller/capacity/builder/report_test.go b/pkg/controller/capacity/builder/report_test.go index f5f1cd9d9..a3785f90b 100644 --- a/pkg/controller/capacity/builder/report_test.go +++ b/pkg/controller/capacity/builder/report_test.go @@ -1,9 +1,10 @@ package builder import ( + "testing" + "github.com/pmezard/go-difflib/difflib" "gopkg.in/yaml.v2" - "testing" shipper "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" ) @@ -46,7 +47,7 @@ func TestReportOneContainerOnePodOneCondition(t *testing.T) { actual := NewReport(ownerName). AddPodConditionBreakdownBuilder( NewPodConditionBreakdown(1, "Ready", "True", ""). - AddContainerState("app", 1, "pod-a", "Ready", "", "")). + AddOrIncrementContainerState("app", "pod-a", "Ready", "", "")). Build() expected := &shipper.ClusterCapacityReport{ Owner: shipper.ClusterCapacityReportOwner{Name: ownerName}, @@ -87,7 +88,7 @@ func TestReportOneContainerOnePodOneConditionTerminatedWithExitCodeContainer(t * actual := NewReport(ownerName). AddPodConditionBreakdownBuilder( NewPodConditionBreakdown(1, "Ready", "True", ""). - AddContainerState("app", 1, "pod-a", "Ready", "", "Terminated with exit code 1")). + AddOrIncrementContainerState("app", "pod-a", "Ready", "", "Terminated with exit code 1")). Build() m := "Terminated with exit code 1" @@ -133,8 +134,8 @@ func TestReportOneContainerTwoPodsOneCondition(t *testing.T) { actual := NewReport(ownerName). AddPodConditionBreakdownBuilder( NewPodConditionBreakdown(2, "Ready", "True", ""). - AddContainerState("app", 1, "pod-a", "Ready", "", ""). - AddContainerState("app", 1, "pod-b", "Ready", "", "")). + AddOrIncrementContainerState("app", "pod-a", "Ready", "", ""). + AddOrIncrementContainerState("app", "pod-b", "Ready", "", "")). Build() expected := &shipper.ClusterCapacityReport{ @@ -176,10 +177,10 @@ func TestReportTwoContainersTwoPodsOneCondition(t *testing.T) { actual := NewReport(ownerName). AddPodConditionBreakdownBuilder( NewPodConditionBreakdown(2, "Ready", "True", ""). - AddContainerState("app", 1, "pod-a", "Ready", "", ""). - AddContainerState("app", 1, "pod-b", "Ready", "", ""). - AddContainerState("nginx", 1, "pod-a", "Ready", "", ""). - AddContainerState("nginx", 1, "pod-b", "Ready", "", "")). + AddOrIncrementContainerState("app", "pod-a", "Ready", "", ""). + AddOrIncrementContainerState("app", "pod-b", "Ready", "", ""). + AddOrIncrementContainerState("nginx", "pod-a", "Ready", "", ""). + AddOrIncrementContainerState("nginx", "pod-b", "Ready", "", "")). Build() expected := &shipper.ClusterCapacityReport{ @@ -234,15 +235,15 @@ func TestReportTwoContainersTwoPodsTwoConditions(t *testing.T) { actual := NewReport(ownerName). AddPodConditionBreakdownBuilder( NewPodConditionBreakdown(2, "Ready", "True", ""). - AddContainerState("app", 1, "pod-a", "Ready", "", ""). - AddContainerState("app", 1, "pod-b", "Ready", "", ""). - AddContainerState("nginx", 1, "pod-a", "Ready", "", ""). - AddContainerState("nginx", 1, "pod-b", "Ready", "", "")).AddPodConditionBreakdownBuilder( + AddOrIncrementContainerState("app", "pod-a", "Ready", "", ""). + AddOrIncrementContainerState("app", "pod-b", "Ready", "", ""). + AddOrIncrementContainerState("nginx", "pod-a", "Ready", "", ""). + AddOrIncrementContainerState("nginx", "pod-b", "Ready", "", "")).AddPodConditionBreakdownBuilder( NewPodConditionBreakdown(2, "PodInitialized", "True", ""). - AddContainerState("app", 1, "pod-a", "Ready", "", ""). - AddContainerState("app", 1, "pod-b", "Ready", "", ""). - AddContainerState("nginx", 1, "pod-a", "Ready", "", ""). - AddContainerState("nginx", 1, "pod-b", "Ready", "", "")). + AddOrIncrementContainerState("app", "pod-a", "Ready", "", ""). + AddOrIncrementContainerState("app", "pod-b", "Ready", "", ""). + AddOrIncrementContainerState("nginx", "pod-a", "Ready", "", ""). + AddOrIncrementContainerState("nginx", "pod-b", "Ready", "", "")). Build() expected := &shipper.ClusterCapacityReport{ diff --git a/pkg/controller/capacity/capacity_controller_test.go b/pkg/controller/capacity/capacity_controller_test.go index d466f27a3..ab30c6fc0 100644 --- a/pkg/controller/capacity/capacity_controller_test.go +++ b/pkg/controller/capacity/capacity_controller_test.go @@ -76,16 +76,16 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePod(t *testing.T c := builder.NewReport("nginx"). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(1, "ContainersReady", "True", ""). - AddContainerState("app", 1, "pod-a", "Running", "", "")). + AddOrIncrementContainerState("app", "pod-a", "Running", "", "")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(1, "Initialized", "True", ""). - AddContainerState("app", 1, "pod-a", "Running", "", "")). + AddOrIncrementContainerState("app", "pod-a", "Running", "", "")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(1, "PodScheduled", "True", ""). - AddContainerState("app", 1, "pod-a", "Running", "", "")). + AddOrIncrementContainerState("app", "pod-a", "Running", "", "")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(1, "Ready", "True", ""). - AddContainerState("app", 1, "pod-a", "Running", "", "")) + AddOrIncrementContainerState("app", "pod-a", "Running", "", "")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) @@ -136,7 +136,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePodCompletedCont c := builder.NewReport("nginx"). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(1, "ContainersReady", "True", ""). - AddContainerState("app", 1, "pod-a", "Terminated", "Completed", "Terminated with exit code 1")) + AddOrIncrementContainerState("app", "pod-a", "Terminated", "Completed", "Terminated with exit code 1")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) @@ -187,7 +187,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePodTerminatedCon c := builder.NewReport("nginx"). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(1, "ContainersReady", "True", ""). - AddContainerState("app", 1, "pod-a", "Terminated", "Terminated", "Terminated with signal 9")) + AddOrIncrementContainerState("app", "pod-a", "Terminated", "Terminated", "Terminated with signal 9")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) @@ -238,7 +238,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePodRestartedCont c := builder.NewReport("nginx"). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(1, "ContainersReady", "True", ""). - AddContainerState("app", 1, "pod-a", "Terminated", "Terminated", "Terminated with signal 9")) + AddOrIncrementContainerState("app", "pod-a", "Terminated", "Terminated", "Terminated with signal 9")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) @@ -289,7 +289,7 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithSinglePodRestartedCont c := builder.NewReport("nginx"). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(1, "ContainersReady", "True", ""). - AddContainerState("app", 1, "pod-a", "Terminated", "Terminated", "termination message")) + AddOrIncrementContainerState("app", "pod-a", "Terminated", "Terminated", "termination message")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) @@ -351,16 +351,20 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePods(t *testin c := builder.NewReport("nginx"). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(2, "ContainersReady", "True", ""). - AddContainerState("app", 2, "pod-a", "Running", "", "")). + AddOrIncrementContainerState("app", "pod-a", "Running", "", ""). + AddOrIncrementContainerState("app", "pod-a", "Running", "", "")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(2, "Initialized", "True", ""). - AddContainerState("app", 2, "pod-a", "Running", "", "")). + AddOrIncrementContainerState("app", "pod-a", "Running", "", ""). + AddOrIncrementContainerState("app", "pod-a", "Running", "", "")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(2, "PodScheduled", "True", ""). - AddContainerState("app", 2, "pod-a", "Running", "", "")). + AddOrIncrementContainerState("app", "pod-a", "Running", "", ""). + AddOrIncrementContainerState("app", "pod-a", "Running", "", "")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(2, "Ready", "True", ""). - AddContainerState("app", 2, "pod-a", "Running", "", "")) + AddOrIncrementContainerState("app", "pod-a", "Running", "", ""). + AddOrIncrementContainerState("app", "pod-a", "Running", "", "")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy()) @@ -427,16 +431,19 @@ func TestCapacityTargetStatusReturnsCorrectFleetReportWithMultiplePodsWithDiffer c := builder.NewReport("nginx"). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(3, string(corev1.PodInitialized), string(corev1.ConditionTrue), ""). - AddContainerState("app", 2, "pod-a", "Waiting", "ContainerCreating", ""). - AddContainerState("app", 1, "pod-c", "Terminated", "Completed", "Terminated with exit code 0")). + AddOrIncrementContainerState("app", "pod-a", "Waiting", "ContainerCreating", ""). + AddOrIncrementContainerState("app", "pod-a", "Waiting", "ContainerCreating", ""). + AddOrIncrementContainerState("app", "pod-c", "Terminated", "Completed", "Terminated with exit code 0")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(3, string(corev1.PodScheduled), string(corev1.ConditionTrue), ""). - AddContainerState("app", 2, "pod-a", "Waiting", "ContainerCreating", ""). - AddContainerState("app", 1, "pod-c", "Terminated", "Completed", "Terminated with exit code 0")). + AddOrIncrementContainerState("app", "pod-a", "Waiting", "ContainerCreating", ""). + AddOrIncrementContainerState("app", "pod-a", "Waiting", "ContainerCreating", ""). + AddOrIncrementContainerState("app", "pod-c", "Terminated", "Completed", "Terminated with exit code 0")). AddPodConditionBreakdownBuilder( builder.NewPodConditionBreakdown(3, string(corev1.PodReady), string(corev1.ConditionFalse), "ContainersNotReady"). - AddContainerState("app", 2, "pod-a", "Waiting", "ContainerCreating", ""). - AddContainerState("app", 1, "pod-c", "Terminated", "Completed", "Terminated with exit code 0")) + AddOrIncrementContainerState("app", "pod-a", "Waiting", "ContainerCreating", ""). + AddOrIncrementContainerState("app", "pod-a", "Waiting", "ContainerCreating", ""). + AddOrIncrementContainerState("app", "pod-c", "Terminated", "Completed", "Terminated with exit code 0")) f.managementObjects = append(f.managementObjects, capacityTarget.DeepCopy())