Merge pull request #42765 from janetkuo/ds-update-validation-fix

Automatic merge from submit-queue

Add DaemonSet templateGeneration validation and tests, and fix a bunch of validation test errors

For DaemonSet update:
1. Validate that templateGeneration is increased when and only when template is changed
1. Validate that templateGeneration is never decreased
1. Added validation tests for templateGeneration 
1. Fix a bunch of errors in validate tests
   - fake tests: almost all validation test error cases failed on "missing resource version", "name changes", "missing update strategy", "selector/template labels mismatch", not on the real validation we wanted to test
   - some error cases should be success cases

@kargakis @lukaszo @kubernetes/sig-apps-bugs 

*I've verified locally that all DaemonSet e2e tests pass with this change.*
This commit is contained in:
Kubernetes Submit Queue 2017-03-14 12:54:49 -07:00 committed by GitHub
commit a8d8542fc7
2 changed files with 269 additions and 65 deletions

View File

@ -98,10 +98,30 @@ func ValidateDaemonSet(ds *extensions.DaemonSet) field.ErrorList {
// ValidateDaemonSetUpdate tests if required fields in the DaemonSet are set.
func ValidateDaemonSetUpdate(ds, oldDS *extensions.DaemonSet) field.ErrorList {
allErrs := apivalidation.ValidateObjectMetaUpdate(&ds.ObjectMeta, &oldDS.ObjectMeta, field.NewPath("metadata"))
allErrs = append(allErrs, ValidateDaemonSetSpecUpdate(&ds.Spec, &oldDS.Spec, field.NewPath("spec"))...)
allErrs = append(allErrs, ValidateDaemonSetSpec(&ds.Spec, field.NewPath("spec"))...)
return allErrs
}
func ValidateDaemonSetSpecUpdate(newSpec, oldSpec *extensions.DaemonSetSpec, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
// TemplateGeneration shouldn't be decremented
if newSpec.TemplateGeneration < oldSpec.TemplateGeneration {
allErrs = append(allErrs, field.Invalid(fldPath.Child("templateGeneration"), newSpec.TemplateGeneration, "must not be decremented"))
}
// TemplateGeneration should be increased when and only when template is changed
templateUpdated := !reflect.DeepEqual(newSpec.Template, oldSpec.Template)
if newSpec.TemplateGeneration == oldSpec.TemplateGeneration && templateUpdated {
allErrs = append(allErrs, field.Invalid(fldPath.Child("templateGeneration"), newSpec.TemplateGeneration, "must be incremented upon template update"))
} else if newSpec.TemplateGeneration > oldSpec.TemplateGeneration && !templateUpdated {
allErrs = append(allErrs, field.Invalid(fldPath.Child("templateGeneration"), newSpec.TemplateGeneration, "must not be incremented without template update"))
}
return allErrs
}
// validateDaemonSetStatus validates a DaemonSetStatus
func validateDaemonSetStatus(status *extensions.DaemonSetStatus, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

View File

@ -471,11 +471,12 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
invalidPodTemplate := api.PodTemplate{
Template: api.PodTemplateSpec{
Spec: api.PodSpec{
// no containers specified
RestartPolicy: api.RestartPolicyAlways,
DNSPolicy: api.DNSClusterFirst,
},
ObjectMeta: metav1.ObjectMeta{
Labels: invalidSelector,
Labels: validSelector,
},
},
}
@ -489,16 +490,18 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
}
type dsUpdateTest struct {
old extensions.DaemonSet
update extensions.DaemonSet
old extensions.DaemonSet
update extensions.DaemonSet
expectedErrNum int
}
successCases := []dsUpdateTest{
{
successCases := map[string]dsUpdateTest{
"no change": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
@ -507,20 +510,22 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
},
{
"change template and selector": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 2,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
@ -529,20 +534,22 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector2},
Template: validPodTemplateAbc2.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector2},
TemplateGeneration: 3,
Template: validPodTemplateAbc2.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
},
{
"change template": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 3,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
@ -551,20 +558,80 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateNodeSelector.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 4,
Template: validPodTemplateNodeSelector.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
},
"change container image name": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector2},
TemplateGeneration: 2,
Template: validPodTemplateDef.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
},
"change update strategy": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 4,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 4,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.RollingUpdateDaemonSetStrategyType,
RollingUpdate: &extensions.RollingUpdateDaemonSet{
MaxUnavailable: intstr.FromInt(1),
},
},
},
},
},
}
for _, successCase := range successCases {
for testName, successCase := range successCases {
// ResourceVersion is required for updates.
successCase.old.ObjectMeta.ResourceVersion = "1"
successCase.update.ObjectMeta.ResourceVersion = "1"
successCase.update.ObjectMeta.ResourceVersion = "2"
// Check test setup
if successCase.expectedErrNum > 0 {
t.Errorf("%q has incorrect test setup with expectedErrNum %d, expected no error", testName, successCase.expectedErrNum)
}
if len(successCase.old.ObjectMeta.ResourceVersion) == 0 || len(successCase.update.ObjectMeta.ResourceVersion) == 0 {
t.Errorf("%q has incorrect test setup with no resource version set", testName)
}
if errs := ValidateDaemonSetUpdate(&successCase.update, &successCase.old); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
t.Errorf("%q expected no error, but got: %v", testName, errs)
}
}
errorCases := map[string]dsUpdateTest{
@ -572,102 +639,219 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
expectedErrNum: 1,
},
"invalid selector": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "", Namespace: metav1.NamespaceDefault},
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: invalidSelector},
Template: validPodTemplateAbc.Template,
Selector: &metav1.LabelSelector{MatchLabels: invalidSelector},
TemplateGeneration: 1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
expectedErrNum: 1,
},
"invalid pod": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "", Namespace: metav1.NamespaceDefault},
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: invalidPodTemplate.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 2,
Template: invalidPodTemplate.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
expectedErrNum: 1,
},
"change container image": {
"invalid read-write volume": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "", Namespace: metav1.NamespaceDefault},
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateDef.Template,
},
},
},
"read-write volume": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
},
},
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: readWriteVolumePodTemplate.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 2,
Template: readWriteVolumePodTemplate.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
expectedErrNum: 1,
},
"invalid update strategy": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "", Namespace: metav1.NamespaceDefault},
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: invalidSelector},
Template: validPodTemplateAbc.Template,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: 1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: "Random",
},
},
},
expectedErrNum: 1,
},
"negative templateGeneration": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: -1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
TemplateGeneration: -1,
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
expectedErrNum: 1,
},
"decreased templateGeneration": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
TemplateGeneration: 2,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
TemplateGeneration: 1,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
expectedErrNum: 1,
},
"unchanged templateGeneration upon template update": {
old: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
TemplateGeneration: 2,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplateAbc.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
update: extensions.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: extensions.DaemonSetSpec{
TemplateGeneration: 2,
Selector: &metav1.LabelSelector{MatchLabels: validSelector2},
Template: validPodTemplateAbc2.Template,
UpdateStrategy: extensions.DaemonSetUpdateStrategy{
Type: extensions.OnDeleteDaemonSetStrategyType,
},
},
},
expectedErrNum: 1,
},
}
for testName, errorCase := range errorCases {
if errs := ValidateDaemonSetUpdate(&errorCase.update, &errorCase.old); len(errs) == 0 {
t.Errorf("expected failure: %s", testName)
// ResourceVersion is required for updates.
errorCase.old.ObjectMeta.ResourceVersion = "1"
errorCase.update.ObjectMeta.ResourceVersion = "2"
// Check test setup
if errorCase.expectedErrNum <= 0 {
t.Errorf("%q has incorrect test setup with expectedErrNum %d, expected at least one error", testName, errorCase.expectedErrNum)
}
if len(errorCase.old.ObjectMeta.ResourceVersion) == 0 || len(errorCase.update.ObjectMeta.ResourceVersion) == 0 {
t.Errorf("%q has incorrect test setup with no resource version set", testName)
}
// Run the tests
if errs := ValidateDaemonSetUpdate(&errorCase.update, &errorCase.old); len(errs) != errorCase.expectedErrNum {
t.Errorf("%q expected %d errors, but got %d error: %v", testName, errorCase.expectedErrNum, len(errs), errs)
} else {
t.Logf("(PASS) %q got errors %v", testName, errs)
}
}
}