From f7fa286b65e425e2cf8d78239e33ce5328505f54 Mon Sep 17 00:00:00 2001 From: Michail Kargakis Date: Sat, 25 Feb 2017 13:46:06 +0100 Subject: [PATCH] Add status validation unit tests, validate updatedReplicas --- pkg/api/validation/validation.go | 33 +-- pkg/api/validation/validation_test.go | 119 +++++++++ pkg/apis/extensions/validation/validation.go | 36 +-- .../extensions/validation/validation_test.go | 232 ++++++++++++++++++ 4 files changed, 392 insertions(+), 28 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index c268741360d..73ad0be7958 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -2720,24 +2720,29 @@ func ValidateReplicationControllerUpdate(controller, oldController *api.Replicat // ValidateReplicationControllerStatusUpdate tests if required fields in the replication controller are set. func ValidateReplicationControllerStatusUpdate(controller, oldController *api.ReplicationController) field.ErrorList { allErrs := ValidateObjectMetaUpdate(&controller.ObjectMeta, &oldController.ObjectMeta, field.NewPath("metadata")) - statusPath := field.NewPath("status") - allErrs = append(allErrs, ValidateNonnegativeField(int64(controller.Status.Replicas), statusPath.Child("replicas"))...) - allErrs = append(allErrs, ValidateNonnegativeField(int64(controller.Status.FullyLabeledReplicas), statusPath.Child("fullyLabeledReplicas"))...) - allErrs = append(allErrs, ValidateNonnegativeField(int64(controller.Status.ReadyReplicas), statusPath.Child("readyReplicas"))...) - allErrs = append(allErrs, ValidateNonnegativeField(int64(controller.Status.AvailableReplicas), statusPath.Child("availableReplicas"))...) - allErrs = append(allErrs, ValidateNonnegativeField(int64(controller.Status.ObservedGeneration), statusPath.Child("observedGeneration"))...) + allErrs = append(allErrs, ValidateReplicationControllerStatus(controller.Status, field.NewPath("status"))...) + return allErrs +} + +func ValidateReplicationControllerStatus(status api.ReplicationControllerStatus, statusPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + allErrs = append(allErrs, ValidateNonnegativeField(int64(status.Replicas), statusPath.Child("replicas"))...) + allErrs = append(allErrs, ValidateNonnegativeField(int64(status.FullyLabeledReplicas), statusPath.Child("fullyLabeledReplicas"))...) + allErrs = append(allErrs, ValidateNonnegativeField(int64(status.ReadyReplicas), statusPath.Child("readyReplicas"))...) + allErrs = append(allErrs, ValidateNonnegativeField(int64(status.AvailableReplicas), statusPath.Child("availableReplicas"))...) + allErrs = append(allErrs, ValidateNonnegativeField(int64(status.ObservedGeneration), statusPath.Child("observedGeneration"))...) msg := "cannot be greater than status.replicas" - if controller.Status.FullyLabeledReplicas > controller.Status.Replicas { - allErrs = append(allErrs, field.Invalid(statusPath.Child("fullyLabeledReplicas"), controller.Status.FullyLabeledReplicas, msg)) + if status.FullyLabeledReplicas > status.Replicas { + allErrs = append(allErrs, field.Invalid(statusPath.Child("fullyLabeledReplicas"), status.FullyLabeledReplicas, msg)) } - if controller.Status.ReadyReplicas > controller.Status.Replicas { - allErrs = append(allErrs, field.Invalid(statusPath.Child("readyReplicas"), controller.Status.ReadyReplicas, msg)) + if status.ReadyReplicas > status.Replicas { + allErrs = append(allErrs, field.Invalid(statusPath.Child("readyReplicas"), status.ReadyReplicas, msg)) } - if controller.Status.AvailableReplicas > controller.Status.Replicas { - allErrs = append(allErrs, field.Invalid(statusPath.Child("availableReplicas"), controller.Status.AvailableReplicas, msg)) + if status.AvailableReplicas > status.Replicas { + allErrs = append(allErrs, field.Invalid(statusPath.Child("availableReplicas"), status.AvailableReplicas, msg)) } - if controller.Status.AvailableReplicas > controller.Status.ReadyReplicas { - allErrs = append(allErrs, field.Invalid(statusPath.Child("availableReplicas"), controller.Status.AvailableReplicas, "cannot be greater than readyReplicas")) + if status.AvailableReplicas > status.ReadyReplicas { + allErrs = append(allErrs, field.Invalid(statusPath.Child("availableReplicas"), status.AvailableReplicas, "cannot be greater than readyReplicas")) } return allErrs } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 7a9153f412f..dbbc7e4f888 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -5318,6 +5318,125 @@ func TestValidateService(t *testing.T) { } } +func TestValidateReplicationControllerStatus(t *testing.T) { + tests := []struct { + name string + + replicas int32 + fullyLabeledReplicas int32 + readyReplicas int32 + availableReplicas int32 + observedGeneration int64 + + expectedErr bool + }{ + { + name: "valid status", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: 2, + availableReplicas: 1, + observedGeneration: 2, + expectedErr: false, + }, + { + name: "invalid replicas", + replicas: -1, + fullyLabeledReplicas: 3, + readyReplicas: 2, + availableReplicas: 1, + observedGeneration: 2, + expectedErr: true, + }, + { + name: "invalid fullyLabeledReplicas", + replicas: 3, + fullyLabeledReplicas: -1, + readyReplicas: 2, + availableReplicas: 1, + observedGeneration: 2, + expectedErr: true, + }, + { + name: "invalid readyReplicas", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: -1, + availableReplicas: 1, + observedGeneration: 2, + expectedErr: true, + }, + { + name: "invalid availableReplicas", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: 3, + availableReplicas: -1, + observedGeneration: 2, + expectedErr: true, + }, + { + name: "invalid observedGeneration", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: 3, + availableReplicas: 3, + observedGeneration: -1, + expectedErr: true, + }, + { + name: "fullyLabeledReplicas greater than replicas", + replicas: 3, + fullyLabeledReplicas: 4, + readyReplicas: 3, + availableReplicas: 3, + observedGeneration: 1, + expectedErr: true, + }, + { + name: "readyReplicas greater than replicas", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: 4, + availableReplicas: 3, + observedGeneration: 1, + expectedErr: true, + }, + { + name: "availableReplicas greater than replicas", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: 3, + availableReplicas: 4, + observedGeneration: 1, + expectedErr: true, + }, + { + name: "availableReplicas greater than readyReplicas", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: 2, + availableReplicas: 3, + observedGeneration: 1, + expectedErr: true, + }, + } + + for _, test := range tests { + status := api.ReplicationControllerStatus{ + Replicas: test.replicas, + FullyLabeledReplicas: test.fullyLabeledReplicas, + ReadyReplicas: test.readyReplicas, + AvailableReplicas: test.availableReplicas, + ObservedGeneration: test.observedGeneration, + } + + if hasErr := len(ValidateReplicationControllerStatus(status, field.NewPath("status"))) > 0; hasErr != test.expectedErr { + t.Errorf("%s: expected error: %t, got error: %t", test.name, test.expectedErr, hasErr) + } + } +} + func TestValidateReplicationControllerStatusUpdate(t *testing.T) { validSelector := map[string]string{"a": "b"} validPodTemplate := api.PodTemplate{ diff --git a/pkg/apis/extensions/validation/validation.go b/pkg/apis/extensions/validation/validation.go index 55b1aefda3a..5ca7a9b1073 100644 --- a/pkg/apis/extensions/validation/validation.go +++ b/pkg/apis/extensions/validation/validation.go @@ -286,6 +286,9 @@ func ValidateDeploymentStatus(status *extensions.DeploymentStatus, fldPath *fiel allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.AvailableReplicas), fldPath.Child("availableReplicas"))...) allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.UnavailableReplicas), fldPath.Child("unavailableReplicas"))...) msg := "cannot be greater than status.replicas" + if status.UpdatedReplicas > status.Replicas { + allErrs = append(allErrs, field.Invalid(fldPath.Child("updatedReplicas"), status.UpdatedReplicas, msg)) + } if status.ReadyReplicas > status.Replicas { allErrs = append(allErrs, field.Invalid(fldPath.Child("readyReplicas"), status.ReadyReplicas, msg)) } @@ -511,25 +514,30 @@ func ValidateReplicaSetUpdate(rs, oldRs *extensions.ReplicaSet) field.ErrorList // ValidateReplicaSetStatusUpdate tests if required fields in the ReplicaSet are set. func ValidateReplicaSetStatusUpdate(rs, oldRs *extensions.ReplicaSet) field.ErrorList { allErrs := field.ErrorList{} - fldPath := field.NewPath("status") allErrs = append(allErrs, apivalidation.ValidateObjectMetaUpdate(&rs.ObjectMeta, &oldRs.ObjectMeta, field.NewPath("metadata"))...) - allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(rs.Status.Replicas), fldPath.Child("replicas"))...) - allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(rs.Status.FullyLabeledReplicas), fldPath.Child("fullyLabeledReplicas"))...) - allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(rs.Status.ReadyReplicas), fldPath.Child("readyReplicas"))...) - allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(rs.Status.AvailableReplicas), fldPath.Child("availableReplicas"))...) - allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(rs.Status.ObservedGeneration), fldPath.Child("observedGeneration"))...) + allErrs = append(allErrs, ValidateReplicaSetStatus(rs.Status, field.NewPath("status"))...) + return allErrs +} + +func ValidateReplicaSetStatus(status extensions.ReplicaSetStatus, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.Replicas), fldPath.Child("replicas"))...) + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.FullyLabeledReplicas), fldPath.Child("fullyLabeledReplicas"))...) + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.ReadyReplicas), fldPath.Child("readyReplicas"))...) + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.AvailableReplicas), fldPath.Child("availableReplicas"))...) + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.ObservedGeneration), fldPath.Child("observedGeneration"))...) msg := "cannot be greater than status.replicas" - if rs.Status.FullyLabeledReplicas > rs.Status.Replicas { - allErrs = append(allErrs, field.Invalid(fldPath.Child("fullyLabeledReplicas"), rs.Status.FullyLabeledReplicas, msg)) + if status.FullyLabeledReplicas > status.Replicas { + allErrs = append(allErrs, field.Invalid(fldPath.Child("fullyLabeledReplicas"), status.FullyLabeledReplicas, msg)) } - if rs.Status.ReadyReplicas > rs.Status.Replicas { - allErrs = append(allErrs, field.Invalid(fldPath.Child("readyReplicas"), rs.Status.ReadyReplicas, msg)) + if status.ReadyReplicas > status.Replicas { + allErrs = append(allErrs, field.Invalid(fldPath.Child("readyReplicas"), status.ReadyReplicas, msg)) } - if rs.Status.AvailableReplicas > rs.Status.Replicas { - allErrs = append(allErrs, field.Invalid(fldPath.Child("availableReplicas"), rs.Status.AvailableReplicas, msg)) + if status.AvailableReplicas > status.Replicas { + allErrs = append(allErrs, field.Invalid(fldPath.Child("availableReplicas"), status.AvailableReplicas, msg)) } - if rs.Status.AvailableReplicas > rs.Status.ReadyReplicas { - allErrs = append(allErrs, field.Invalid(fldPath.Child("availableReplicas"), rs.Status.AvailableReplicas, "cannot be greater than readyReplicas")) + if status.AvailableReplicas > status.ReadyReplicas { + allErrs = append(allErrs, field.Invalid(fldPath.Child("availableReplicas"), status.AvailableReplicas, "cannot be greater than readyReplicas")) } return allErrs } diff --git a/pkg/apis/extensions/validation/validation_test.go b/pkg/apis/extensions/validation/validation_test.go index 2584ec11740..fafede1a6af 100644 --- a/pkg/apis/extensions/validation/validation_test.go +++ b/pkg/apis/extensions/validation/validation_test.go @@ -836,6 +836,119 @@ func TestValidateDeployment(t *testing.T) { } } +func TestValidateDeploymentStatus(t *testing.T) { + tests := []struct { + name string + + replicas int32 + updatedReplicas int32 + readyReplicas int32 + availableReplicas int32 + observedGeneration int64 + + expectedErr bool + }{ + { + name: "valid status", + replicas: 3, + updatedReplicas: 3, + readyReplicas: 2, + availableReplicas: 1, + observedGeneration: 2, + expectedErr: false, + }, + { + name: "invalid replicas", + replicas: -1, + updatedReplicas: 2, + readyReplicas: 2, + availableReplicas: 1, + observedGeneration: 2, + expectedErr: true, + }, + { + name: "invalid updatedReplicas", + replicas: 2, + updatedReplicas: -1, + readyReplicas: 2, + availableReplicas: 1, + observedGeneration: 2, + expectedErr: true, + }, + { + name: "invalid readyReplicas", + replicas: 3, + readyReplicas: -1, + availableReplicas: 1, + observedGeneration: 2, + expectedErr: true, + }, + { + name: "invalid availableReplicas", + replicas: 3, + readyReplicas: 3, + availableReplicas: -1, + observedGeneration: 2, + expectedErr: true, + }, + { + name: "invalid observedGeneration", + replicas: 3, + readyReplicas: 3, + availableReplicas: 3, + observedGeneration: -1, + expectedErr: true, + }, + { + name: "updatedReplicas greater than replicas", + replicas: 3, + updatedReplicas: 4, + readyReplicas: 3, + availableReplicas: 3, + observedGeneration: 1, + expectedErr: true, + }, + { + name: "readyReplicas greater than replicas", + replicas: 3, + readyReplicas: 4, + availableReplicas: 3, + observedGeneration: 1, + expectedErr: true, + }, + { + name: "availableReplicas greater than replicas", + replicas: 3, + readyReplicas: 3, + availableReplicas: 4, + observedGeneration: 1, + expectedErr: true, + }, + { + name: "availableReplicas greater than readyReplicas", + replicas: 3, + readyReplicas: 2, + availableReplicas: 3, + observedGeneration: 1, + expectedErr: true, + }, + } + + for _, test := range tests { + status := extensions.DeploymentStatus{ + Replicas: test.replicas, + UpdatedReplicas: test.updatedReplicas, + ReadyReplicas: test.readyReplicas, + AvailableReplicas: test.availableReplicas, + ObservedGeneration: test.observedGeneration, + } + + if hasErr := len(ValidateDeploymentStatus(&status, field.NewPath("status"))) > 0; hasErr != test.expectedErr { + t.Errorf("%s: expected error: %t, got error: %t", test.name, test.expectedErr, hasErr) + } + } +} + func validDeploymentRollback() *extensions.DeploymentRollback { return &extensions.DeploymentRollback{ Name: "abc", @@ -1208,6 +1321,125 @@ func TestValidateScale(t *testing.T) { } } +func TestValidateReplicaSetStatus(t *testing.T) { + tests := []struct { + name string + + replicas int32 + fullyLabeledReplicas int32 + readyReplicas int32 + availableReplicas int32 + observedGeneration int64 + + expectedErr bool + }{ + { + name: "valid status", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: 2, + availableReplicas: 1, + observedGeneration: 2, + expectedErr: false, + }, + { + name: "invalid replicas", + replicas: -1, + fullyLabeledReplicas: 3, + readyReplicas: 2, + availableReplicas: 1, + observedGeneration: 2, + expectedErr: true, + }, + { + name: "invalid fullyLabeledReplicas", + replicas: 3, + fullyLabeledReplicas: -1, + readyReplicas: 2, + availableReplicas: 1, + observedGeneration: 2, + expectedErr: true, + }, + { + name: "invalid readyReplicas", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: -1, + availableReplicas: 1, + observedGeneration: 2, + expectedErr: true, + }, + { + name: "invalid availableReplicas", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: 3, + availableReplicas: -1, + observedGeneration: 2, + expectedErr: true, + }, + { + name: "invalid observedGeneration", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: 3, + availableReplicas: 3, + observedGeneration: -1, + expectedErr: true, + }, + { + name: "fullyLabeledReplicas greater than replicas", + replicas: 3, + fullyLabeledReplicas: 4, + readyReplicas: 3, + availableReplicas: 3, + observedGeneration: 1, + expectedErr: true, + }, + { + name: "readyReplicas greater than replicas", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: 4, + availableReplicas: 3, + observedGeneration: 1, + expectedErr: true, + }, + { + name: "availableReplicas greater than replicas", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: 3, + availableReplicas: 4, + observedGeneration: 1, + expectedErr: true, + }, + { + name: "availableReplicas greater than readyReplicas", + replicas: 3, + fullyLabeledReplicas: 3, + readyReplicas: 2, + availableReplicas: 3, + observedGeneration: 1, + expectedErr: true, + }, + } + + for _, test := range tests { + status := extensions.ReplicaSetStatus{ + Replicas: test.replicas, + FullyLabeledReplicas: test.fullyLabeledReplicas, + ReadyReplicas: test.readyReplicas, + AvailableReplicas: test.availableReplicas, + ObservedGeneration: test.observedGeneration, + } + + if hasErr := len(ValidateReplicaSetStatus(status, field.NewPath("status"))) > 0; hasErr != test.expectedErr { + t.Errorf("%s: expected error: %t, got error: %t", test.name, test.expectedErr, hasErr) + } + } +} + func TestValidateReplicaSetStatusUpdate(t *testing.T) { validLabels := map[string]string{"a": "b"} validPodTemplate := api.PodTemplate{