Honor OS update policy at InstanceGroup level too

As with the Cluster-level "spec.updatePolicy" field, add a similar
field at the InstanceGroup level, allowing overriding of the
cluster-level choice in each InstanceGroup.

Introduce a new value for the field ("automatic") as equivalent to the
default value applied when the field is absent. Honoring this new
value allows disabling automatic updates at the cluster level, but
then enabling them again for particular InstanceGroups. Without such a
positive affirmation, it's not possible to override a cluster-level
"external" policy at the InstanceGroup level, as there's no way to
specify positively that you want to recover the default
value. Instead, expressing the explicit "automatic" value is clear and
unambiguous.

k8s/crds/kops.k8s.io_clusters.yaml

@@ -4070,10 +4070,10 @@ spec:
                 type: object
               updatePolicy:
                 description: 'UpdatePolicy determines the policy for applying upgrades
-                  automatically. Valid values:   ''external'' do not apply updates
-                  automatically - they are applied manually or by an external system   missing:
-                  default policy (currently OS security upgrades that do not require
-                  a reboot)'
+                  automatically. Valid values:   ''automatic'' (default): apply updates
+                  automatically (apply OS security upgrades, avoiding rebooting when
+                  possible)   ''external'': do not apply updates automatically; they
+                  are applied manually or by an external system'
                 type: string
               useHostCertificates:
                 description: UseHostCertificates will mount /etc/ssl/certs to inside

k8s/crds/kops.k8s.io_instancegroups.yaml

@@ -825,6 +825,13 @@ spec:
                 description: Describes the tenancy of the instance group. Can be either
                   default or dedicated. Currently only applies to AWS.
                 type: string
+              updatePolicy:
+                description: 'UpdatePolicy determines the policy for applying upgrades
+                  automatically. Valid values:   ''automatic'' (default): apply updates
+                  automatically (apply OS security upgrades, avoiding rebooting when
+                  possible)   ''external'': do not apply updates automatically; they
+                  are applied manually or by an external system'
+                type: string
               volumeMounts:
                 description: VolumeMounts a collection of volume mounts
                 items:

nodeup/pkg/model/update_service.go

@@ -49,8 +49,16 @@ func (b *UpdateServiceBuilder) Build(c *fi.ModelBuilderContext) error {
 }
 
 func (b *UpdateServiceBuilder) buildFlatcarSystemdService(c *fi.ModelBuilderContext) {
-	if b.Cluster.Spec.UpdatePolicy == nil || *b.Cluster.Spec.UpdatePolicy != kops.UpdatePolicyExternal {
-		klog.Infof("UpdatePolicy not set in Cluster Spec; skipping creation of %s", flatcarServiceName)
+	if b.InstanceGroup.Spec.UpdatePolicy != nil {
+		switch *b.InstanceGroup.Spec.UpdatePolicy {
+		case kops.UpdatePolicyAutomatic:
+			klog.Infof("UpdatePolicy set in InstanceGroup %q spec requests automatic updates; skipping creation of systemd unit %q", b.InstanceGroup.GetName(), flatcarServiceName)
+			return
+		case kops.UpdatePolicyExternal:
+			// Carry on with creating this systemd unit.
+		}
+	} else if fi.StringValue(b.Cluster.Spec.UpdatePolicy) != kops.UpdatePolicyExternal {
+		klog.Infof("UpdatePolicy in Cluster spec requests automatic updates; skipping creation of systemd unit %q", flatcarServiceName)
 		return
 	}
 
@@ -85,8 +93,16 @@ func (b *UpdateServiceBuilder) buildFlatcarSystemdService(c *fi.ModelBuilderCont
 }
 
 func (b *UpdateServiceBuilder) buildDebianPackage(c *fi.ModelBuilderContext) {
-	if b.Cluster.Spec.UpdatePolicy != nil && *b.Cluster.Spec.UpdatePolicy == kops.UpdatePolicyExternal {
-		klog.Infof("UpdatePolicy is External; skipping installation of %s", debianPackageName)
+	if b.InstanceGroup.Spec.UpdatePolicy != nil {
+		switch *b.InstanceGroup.Spec.UpdatePolicy {
+		case kops.UpdatePolicyAutomatic:
+			klog.Infof("UpdatePolicy set in InstanceGroup %q spec requests automatic updates; skipping installation of packagk %q", b.InstanceGroup.GetName(), debianPackageName)
+			return
+		case kops.UpdatePolicyExternal:
+			// Carry on with creating this systemd unit.
+		}
+	} else if fi.StringValue(b.Cluster.Spec.UpdatePolicy) != kops.UpdatePolicyExternal {
+		klog.Infof("UpdatePolicy in Cluster spec requests automatic updates; skipping installation of package %q", debianPackageName)
 		return
 	}
 

pkg/apis/kops/cluster.go

@@ -133,8 +133,8 @@ type ClusterSpec struct {
 	IsolateMasters *bool `json:"isolateMasters,omitempty"`
 	// UpdatePolicy determines the policy for applying upgrades automatically.
 	// Valid values:
-	//   'external' do not apply updates automatically - they are applied manually or by an external system
-	//   missing: default policy (currently OS security upgrades that do not require a reboot)
+	//   'automatic' (default): apply updates automatically (apply OS security upgrades, avoiding rebooting when possible)
+	//   'external': do not apply updates automatically; they are applied manually or by an external system
 	UpdatePolicy *string `json:"updatePolicy,omitempty"`
 	// ExternalPolicies allows the insertion of pre-existing managed policies on IG Roles
 	ExternalPolicies *map[string][]string `json:"externalPolicies,omitempty"`

pkg/apis/kops/instancegroup.go

@@ -176,6 +176,11 @@ type InstanceGroupSpec struct {
 	CompressUserData *bool `json:"compressUserData,omitempty"`
 	// InstanceMetadata defines the EC2 instance metadata service options (AWS Only)
 	InstanceMetadata *InstanceMetadataOptions `json:"instanceMetadata,omitempty"`
+	// UpdatePolicy determines the policy for applying upgrades automatically.
+	// Valid values:
+	//   'automatic' (default): apply updates automatically (apply OS security upgrades, avoiding rebooting when possible)
+	//   'external': do not apply updates automatically; they are applied manually or by an external system
+	UpdatePolicy *string `json:"updatePolicy,omitempty"`
 }
 
 const (
@@ -190,7 +195,7 @@ const (
 // SpotAllocationStrategies is a collection of supported strategies
 var SpotAllocationStrategies = []string{SpotAllocationStrategyLowestPrices, SpotAllocationStrategyDiversified, SpotAllocationStrategyCapacityOptimized}
 
-// InstanceMetadata defines the EC2 instance metadata service options (AWS Only)
+// InstanceMetadataOptions defines the EC2 instance metadata service options (AWS Only)
 type InstanceMetadataOptions struct {
 	// HTTPPutResponseHopLimit is the desired HTTP PUT response hop limit for instance metadata requests.
 	// The larger the number, the further instance metadata requests can travel. The default value is 1.

pkg/apis/kops/labels.go

@@ -23,6 +23,9 @@ const (
 	// AnnotationValueManagementImported is the annotation value that indicates a cluster was imported, typically as part of an upgrade
 	AnnotationValueManagementImported = "imported"
 
-	// UpdatePolicyExternal is a value for ClusterSpec.UpdatePolicy indicating that upgrades are done externally, and we should disable automatic upgrades
+	// UpdatePolicyAutomatic is a value for ClusterSpec.UpdatePolicy and InstanceGroup.UpdatePolicy indicating that upgrades are performed automatically
+	UpdatePolicyAutomatic = "automatic"
+
+	// UpdatePolicyExternal is a value for ClusterSpec.UpdatePolicy and InstanceGroup.UpdatePolicy indicating that upgrades are done externally, and we should disable automatic upgrades
 	UpdatePolicyExternal = "external"
 )

pkg/apis/kops/v1alpha2/cluster.go

@@ -132,8 +132,8 @@ type ClusterSpec struct {
 	IsolateMasters *bool `json:"isolateMasters,omitempty"`
 	// UpdatePolicy determines the policy for applying upgrades automatically.
 	// Valid values:
-	//   'external' do not apply updates automatically - they are applied manually or by an external system
-	//   missing: default policy (currently OS security upgrades that do not require a reboot)
+	//   'automatic' (default): apply updates automatically (apply OS security upgrades, avoiding rebooting when possible)
+	//   'external': do not apply updates automatically; they are applied manually or by an external system
 	UpdatePolicy *string `json:"updatePolicy,omitempty"`
 	// ExternalPolicies allows the insertion of pre-existing managed policies on IG Roles
 	ExternalPolicies *map[string][]string `json:"externalPolicies,omitempty"`

pkg/apis/kops/v1alpha2/instancegroup.go

@@ -174,6 +174,11 @@ type InstanceGroupSpec struct {
 	CompressUserData *bool `json:"compressUserData,omitempty"`
 	// InstanceMetadata defines the EC2 instance metadata service options (AWS Only)
 	InstanceMetadata *InstanceMetadataOptions `json:"instanceMetadata,omitempty"`
+	// UpdatePolicy determines the policy for applying upgrades automatically.
+	// Valid values:
+	//   'automatic' (default): apply updates automatically (apply OS security upgrades, avoiding rebooting when possible)
+	//   'external': do not apply updates automatically; they are applied manually or by an external system
+	UpdatePolicy *string `json:"updatePolicy,omitempty"`
 }
 
 const (
@@ -188,7 +193,7 @@ const (
 // SpotAllocationStrategies is a collection of supported strategies
 var SpotAllocationStrategies = []string{SpotAllocationStrategyLowestPrices, SpotAllocationStrategyDiversified, SpotAllocationStrategyCapacityOptimized}
 
-// InstanceMetadata defines the EC2 instance metadata service options (AWS Only)
+// InstanceMetadataOptions defines the EC2 instance metadata service options (AWS Only)
 type InstanceMetadataOptions struct {
 	// HTTPPutResponseHopLimit is the desired HTTP PUT response hop limit for instance metadata requests.
 	// The larger the number, the further instance metadata requests can travel. The default value is 1.

pkg/apis/kops/v1alpha2/zz_generated.conversion.go

@@ -3952,6 +3952,7 @@ func autoConvert_v1alpha2_InstanceGroupSpec_To_kops_InstanceGroupSpec(in *Instan
 	} else {
 		out.InstanceMetadata = nil
 	}
+	out.UpdatePolicy = in.UpdatePolicy
 	return nil
 }
 
@@ -4104,6 +4105,7 @@ func autoConvert_kops_InstanceGroupSpec_To_v1alpha2_InstanceGroupSpec(in *kops.I
 	} else {
 		out.InstanceMetadata = nil
 	}
+	out.UpdatePolicy = in.UpdatePolicy
 	return nil
 }
 

pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go

@@ -2174,6 +2174,11 @@ func (in *InstanceGroupSpec) DeepCopyInto(out *InstanceGroupSpec) {
 		*out = new(InstanceMetadataOptions)
 		(*in).DeepCopyInto(*out)
 	}
+	if in.UpdatePolicy != nil {
+		in, out := &in.UpdatePolicy, &out.UpdatePolicy
+		*out = new(string)
+		**out = **in
+	}
 	return
 }
 

pkg/apis/kops/validation/instancegroup.go

@@ -140,6 +140,8 @@ func ValidateInstanceGroup(g *kops.InstanceGroup, cloud fi.Cloud) field.ErrorLis
 		allErrs = append(allErrs, validateExternalLoadBalancer(&lb, path)...)
 	}
 
+	allErrs = append(allErrs, IsValidValue(field.NewPath("spec", "updatePolicy"), g.Spec.UpdatePolicy, []string{kops.UpdatePolicyAutomatic, kops.UpdatePolicyExternal})...)
+
 	return allErrs
 }
 

pkg/apis/kops/validation/instancegroup_test.go

@@ -338,3 +338,49 @@ func TestIGCloudLabelIsIGName(t *testing.T) {
 		testErrors(t, g.label, errs, g.expected)
 	}
 }
+
+func TestIGUpdatePolicy(t *testing.T) {
+	const unsupportedValueError = "Unsupported value::spec.updatePolicy"
+	for _, test := range []struct {
+		label    string
+		policy   *string
+		expected []string
+	}{
+		{
+			label: "missing",
+		},
+		{
+			label:  "automatic",
+			policy: fi.String(kops.UpdatePolicyAutomatic),
+		},
+		{
+			label:  "external",
+			policy: fi.String(kops.UpdatePolicyExternal),
+		},
+		{
+			label:    "empty",
+			policy:   fi.String(""),
+			expected: []string{unsupportedValueError},
+		},
+		{
+			label:    "unknown",
+			policy:   fi.String("something-else"),
+			expected: []string{unsupportedValueError},
+		},
+	} {
+		ig := kops.InstanceGroup{
+			ObjectMeta: v1.ObjectMeta{
+				Name: "some-ig",
+			},
+			Spec: kops.InstanceGroupSpec{
+				Role:        "Node",
+				CloudLabels: make(map[string]string),
+			},
+		}
+		t.Run(test.label, func(t *testing.T) {
+			ig.Spec.UpdatePolicy = test.policy
+			errs := ValidateInstanceGroup(&ig, nil)
+			testErrors(t, test.label, errs, test.expected)
+		})
+	}
+}

pkg/apis/kops/validation/validation.go

@@ -105,7 +105,7 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie
 	}
 
 	// UpdatePolicy
-	allErrs = append(allErrs, IsValidValue(fieldPath.Child("updatePolicy"), spec.UpdatePolicy, []string{kops.UpdatePolicyExternal})...)
+	allErrs = append(allErrs, IsValidValue(fieldPath.Child("updatePolicy"), spec.UpdatePolicy, []string{kops.UpdatePolicyAutomatic, kops.UpdatePolicyExternal})...)
 
 	// Hooks
 	for i := range spec.Hooks {

pkg/apis/kops/zz_generated.deepcopy.go

@@ -2340,6 +2340,11 @@ func (in *InstanceGroupSpec) DeepCopyInto(out *InstanceGroupSpec) {
 		*out = new(InstanceMetadataOptions)
 		(*in).DeepCopyInto(*out)
 	}
+	if in.UpdatePolicy != nil {
+		in, out := &in.UpdatePolicy, &out.UpdatePolicy
+		*out = new(string)
+		**out = **in
+	}
 	return
 }
 

upup/pkg/fi/cloudup/validation_test.go

@@ -112,14 +112,49 @@ func TestValidateFull_ClusterName_Required(t *testing.T) {
 
 func TestValidateFull_UpdatePolicy_Valid(t *testing.T) {
 	c := buildDefaultCluster(t)
-	c.Spec.UpdatePolicy = fi.String(api.UpdatePolicyExternal)
-	expectNoErrorFromValidate(t, c)
+	for _, test := range []struct {
+		label  string
+		policy *string
+	}{
+		{
+			label: "missing",
+		},
+		{
+			label:  "automatic",
+			policy: fi.String(api.UpdatePolicyAutomatic),
+		},
+		{
+			label:  "external",
+			policy: fi.String(api.UpdatePolicyExternal),
+		},
+	} {
+		t.Run(test.label, func(t *testing.T) {
+			c.Spec.UpdatePolicy = test.policy
+			expectNoErrorFromValidate(t, c)
+		})
+	}
 }
 
 func TestValidateFull_UpdatePolicy_Invalid(t *testing.T) {
 	c := buildDefaultCluster(t)
-	c.Spec.UpdatePolicy = fi.String("not-a-real-value")
-	expectErrorFromValidate(t, c, "spec.updatePolicy")
+	for _, test := range []struct {
+		label  string
+		policy string
+	}{
+		{
+			label:  "empty",
+			policy: "",
+		},
+		{
+			label:  "populated",
+			policy: "not-a-real-value",
+		},
+	} {
+		t.Run(test.label, func(t *testing.T) {
+			c.Spec.UpdatePolicy = &test.policy
+			expectErrorFromValidate(t, c, "spec.updatePolicy")
+		})
+	}
 }
 
 func Test_Validate_Kubenet_With_14(t *testing.T) {