From e39c985ee7998baab53886c2035d1689eb986e8d Mon Sep 17 00:00:00 2001 From: "Steven E. Harris" Date: Tue, 23 Feb 2021 11:24:16 -0500 Subject: [PATCH 1/3] 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 | 8 ++-- k8s/crds/kops.k8s.io_instancegroups.yaml | 7 +++ nodeup/pkg/model/update_service.go | 24 ++++++++-- pkg/apis/kops/cluster.go | 4 +- pkg/apis/kops/instancegroup.go | 7 ++- pkg/apis/kops/labels.go | 5 +- pkg/apis/kops/v1alpha2/cluster.go | 4 +- pkg/apis/kops/v1alpha2/instancegroup.go | 7 ++- .../kops/v1alpha2/zz_generated.conversion.go | 2 + .../kops/v1alpha2/zz_generated.deepcopy.go | 5 ++ pkg/apis/kops/validation/instancegroup.go | 2 + .../kops/validation/instancegroup_test.go | 46 +++++++++++++++++++ pkg/apis/kops/validation/validation.go | 2 +- pkg/apis/kops/zz_generated.deepcopy.go | 5 ++ upup/pkg/fi/cloudup/validation_test.go | 43 +++++++++++++++-- 15 files changed, 151 insertions(+), 20 deletions(-) diff --git a/k8s/crds/kops.k8s.io_clusters.yaml b/k8s/crds/kops.k8s.io_clusters.yaml index d83fddd454709..724fbaa031d03 100644 --- a/k8s/crds/kops.k8s.io_clusters.yaml +++ b/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 diff --git a/k8s/crds/kops.k8s.io_instancegroups.yaml b/k8s/crds/kops.k8s.io_instancegroups.yaml index c0111901d3a5b..1516ae79c3beb 100644 --- a/k8s/crds/kops.k8s.io_instancegroups.yaml +++ b/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: diff --git a/nodeup/pkg/model/update_service.go b/nodeup/pkg/model/update_service.go index ae249fd6b820a..d0013259c6f78 100644 --- a/nodeup/pkg/model/update_service.go +++ b/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 } diff --git a/pkg/apis/kops/cluster.go b/pkg/apis/kops/cluster.go index 4d8a8ea570b76..ca3ace733b6f2 100644 --- a/pkg/apis/kops/cluster.go +++ b/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"` diff --git a/pkg/apis/kops/instancegroup.go b/pkg/apis/kops/instancegroup.go index 702b0ed42aa1a..ff2f6540d90e6 100644 --- a/pkg/apis/kops/instancegroup.go +++ b/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. diff --git a/pkg/apis/kops/labels.go b/pkg/apis/kops/labels.go index 18046ebb5341a..affaeb8c991d3 100644 --- a/pkg/apis/kops/labels.go +++ b/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" ) diff --git a/pkg/apis/kops/v1alpha2/cluster.go b/pkg/apis/kops/v1alpha2/cluster.go index 89a6e003173c3..effa77d261816 100644 --- a/pkg/apis/kops/v1alpha2/cluster.go +++ b/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"` diff --git a/pkg/apis/kops/v1alpha2/instancegroup.go b/pkg/apis/kops/v1alpha2/instancegroup.go index d49fcf77da7ac..988dd019f3b9b 100644 --- a/pkg/apis/kops/v1alpha2/instancegroup.go +++ b/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. diff --git a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go index 9c165e45f3584..491bc742c3a47 100644 --- a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go +++ b/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 } diff --git a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go index bc5d769979884..f6fdbfb9fd7f3 100644 --- a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go +++ b/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 } diff --git a/pkg/apis/kops/validation/instancegroup.go b/pkg/apis/kops/validation/instancegroup.go index 858e4a9202767..169ffed7e21e0 100644 --- a/pkg/apis/kops/validation/instancegroup.go +++ b/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 } diff --git a/pkg/apis/kops/validation/instancegroup_test.go b/pkg/apis/kops/validation/instancegroup_test.go index a5a2909fef823..80263d4eb2d8c 100644 --- a/pkg/apis/kops/validation/instancegroup_test.go +++ b/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) + }) + } +} diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index e5162d0f06a30..e4082341b80a9 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/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 { diff --git a/pkg/apis/kops/zz_generated.deepcopy.go b/pkg/apis/kops/zz_generated.deepcopy.go index 6a8a8d6c8c7d3..4b25842d9b66f 100644 --- a/pkg/apis/kops/zz_generated.deepcopy.go +++ b/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 } diff --git a/upup/pkg/fi/cloudup/validation_test.go b/upup/pkg/fi/cloudup/validation_test.go index 7e9c12e03407b..fb3557600c7ea 100644 --- a/upup/pkg/fi/cloudup/validation_test.go +++ b/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) { From 70e95fccf5fc3b7d294b2942587bb50bd3e172d1 Mon Sep 17 00:00:00 2001 From: "Steven E. Harris" Date: Tue, 23 Feb 2021 15:45:54 -0500 Subject: [PATCH 2/3] Copyedit the InstanceGroup-related documentation --- k8s/crds/kops.k8s.io_instancegroups.yaml | 27 ++++++++++++------------ pkg/apis/kops/instancegroup.go | 19 +++++++++-------- pkg/apis/kops/v1alpha2/instancegroup.go | 15 +++++++------ 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/k8s/crds/kops.k8s.io_instancegroups.yaml b/k8s/crds/kops.k8s.io_instancegroups.yaml index 1516ae79c3beb..34578bd1696b2 100644 --- a/k8s/crds/kops.k8s.io_instancegroups.yaml +++ b/k8s/crds/kops.k8s.io_instancegroups.yaml @@ -58,7 +58,7 @@ spec: metadata: type: object spec: - description: InstanceGroupSpec is the specification for an instanceGroup + description: InstanceGroupSpec is the specification for an InstanceGroup properties: additionalSecurityGroups: description: AdditionalSecurityGroups attaches additional security @@ -89,7 +89,7 @@ spec: type: boolean autoscale: description: Autoscale determines if autoscaling will be enabled for - the group if cluster autoscaler is enabled + this instance group if cluster autoscaler is enabled type: boolean cloudLabels: additionalProperties: @@ -111,7 +111,7 @@ spec: type: boolean externalLoadBalancers: description: ExternalLoadBalancers define loadbalancers that should - be attached to the instancegroup + be attached to this instance group items: description: LoadBalancer defines a load balancer properties: @@ -706,11 +706,11 @@ spec: additionalProperties: type: string description: NodeLabels indicates the kubernetes labels for nodes - in this group + in this instance group type: object role: - description: 'Type determines the role of instances in this group: - masters or nodes' + description: 'Type determines the role of instances in this instance + group: masters or nodes' type: string rollingUpdate: description: RollingUpdate defines the rolling-update behavior @@ -817,20 +817,21 @@ spec: type: array taints: description: Taints indicates the kubernetes taints for nodes in this - group + instance group items: type: string type: array tenancy: - description: Describes the tenancy of the instance group. Can be either - default or dedicated. Currently only applies to AWS. + description: Describes the tenancy of this 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' + automatically. If specified, this value overrides a value specified + in the Cluster''s "spec.updatePolicy" field. 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 diff --git a/pkg/apis/kops/instancegroup.go b/pkg/apis/kops/instancegroup.go index ff2f6540d90e6..4abd65ecf352c 100644 --- a/pkg/apis/kops/instancegroup.go +++ b/pkg/apis/kops/instancegroup.go @@ -82,9 +82,9 @@ var ( SupportedFilesystems = []string{BtfsFilesystem, Ext4Filesystem, XFSFilesystem} ) -// InstanceGroupSpec is the specification for a instanceGroup +// InstanceGroupSpec is the specification for an InstanceGroup type InstanceGroupSpec struct { - // Type determines the role of instances in this group: masters or nodes + // Type determines the role of instances in this instance group: masters or nodes Role InstanceGroupRole `json:"role,omitempty"` // Image is the instance (ami etc) we should use Image string `json:"image,omitempty"` @@ -92,7 +92,7 @@ type InstanceGroupSpec struct { MinSize *int32 `json:"minSize,omitempty"` // MaxSize is the maximum size of the pool MaxSize *int32 `json:"maxSize,omitempty"` - // Autoscale determines if autoscaling will be enabled for the group if cluster autoscaler is enabled + // Autoscale determines if autoscaling will be enabled for this instance group if cluster autoscaler is enabled Autoscale *bool `json:"autoscale,omitempty"` // MachineType is the instance class MachineType string `json:"machineType,omitempty"` @@ -114,7 +114,7 @@ type InstanceGroupSpec struct { RootVolumeEncryption *bool `json:"rootVolumeEncryption,omitempty"` // RootVolumeEncryptionKey provides the key identifier for root volume encryption RootVolumeEncryptionKey *string `json:"rootVolumeEncryptionKey,omitempty"` - // Volumes is a collection of additional volumes to create for instances within this InstanceGroup + // Volumes is a collection of additional volumes to create for instances within this instance group Volumes []VolumeSpec `json:"volumes,omitempty"` // VolumeMounts a collection of volume mounts VolumeMounts []VolumeMountSpec `json:"volumeMounts,omitempty"` @@ -123,7 +123,7 @@ type InstanceGroupSpec struct { // Zones is the names of the Zones where machines in this instance group should be placed // This is needed for regional subnets (e.g. GCE), to restrict placement to particular zones Zones []string `json:"zones,omitempty"` - // Hooks is a list of hooks for this instanceGroup, note: these can override the cluster wide ones if required + // Hooks is a list of hooks for this instance group, note: these can override the cluster wide ones if required Hooks []HookSpec `json:"hooks,omitempty"` // MaxPrice indicates this is a spot-pricing group, with the specified value as our max-price bid MaxPrice *string `json:"maxPrice,omitempty"` @@ -137,15 +137,15 @@ type InstanceGroupSpec struct { AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"` // CloudLabels defines additional tags or labels on cloud provider resources CloudLabels map[string]string `json:"cloudLabels,omitempty"` - // NodeLabels indicates the kubernetes labels for nodes in this group + // NodeLabels indicates the kubernetes labels for nodes in this instance group NodeLabels map[string]string `json:"nodeLabels,omitempty"` // FileAssets is a collection of file assets for this instance group FileAssets []FileAssetSpec `json:"fileAssets,omitempty"` - // Describes the tenancy of the instance group. Can be either default or dedicated. Currently only applies to AWS. + // Describes the tenancy of this instance group. Can be either default or dedicated. Currently only applies to AWS. Tenancy string `json:"tenancy,omitempty"` // Kubelet overrides kubelet config from the ClusterSpec Kubelet *KubeletConfigSpec `json:"kubelet,omitempty"` - // Taints indicates the kubernetes taints for nodes in this group + // Taints indicates the kubernetes taints for nodes in this instance group Taints []string `json:"taints,omitempty"` // MixedInstancesPolicy defined a optional backing of an AWS ASG by a EC2 Fleet (AWS Only) MixedInstancesPolicy *MixedInstancesPolicySpec `json:"mixedInstancesPolicy,omitempty"` @@ -153,7 +153,7 @@ type InstanceGroupSpec struct { AdditionalUserData []UserData `json:"additionalUserData,omitempty"` // SuspendProcesses disables the listed Scaling Policies SuspendProcesses []string `json:"suspendProcesses,omitempty"` - // ExternalLoadBalancers define loadbalancers that should be attached to the instancegroup + // ExternalLoadBalancers define loadbalancers that should be attached to this instance group ExternalLoadBalancers []LoadBalancer `json:"externalLoadBalancers,omitempty"` // DetailedInstanceMonitoring defines if detailed-monitoring is enabled (AWS only) DetailedInstanceMonitoring *bool `json:"detailedInstanceMonitoring,omitempty"` @@ -177,6 +177,7 @@ type InstanceGroupSpec struct { // InstanceMetadata defines the EC2 instance metadata service options (AWS Only) InstanceMetadata *InstanceMetadataOptions `json:"instanceMetadata,omitempty"` // UpdatePolicy determines the policy for applying upgrades automatically. + // If specified, this value overrides a value specified in the Cluster's "spec.updatePolicy" field. // 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 diff --git a/pkg/apis/kops/v1alpha2/instancegroup.go b/pkg/apis/kops/v1alpha2/instancegroup.go index 988dd019f3b9b..5751eb0c7b1c4 100644 --- a/pkg/apis/kops/v1alpha2/instancegroup.go +++ b/pkg/apis/kops/v1alpha2/instancegroup.go @@ -79,9 +79,9 @@ var ( SupportedFilesystems = []string{BtfsFilesystem, Ext4Filesystem, XFSFilesystem} ) -// InstanceGroupSpec is the specification for an instanceGroup +// InstanceGroupSpec is the specification for an InstanceGroup type InstanceGroupSpec struct { - // Type determines the role of instances in this group: masters or nodes + // Type determines the role of instances in this instance group: masters or nodes Role InstanceGroupRole `json:"role,omitempty"` // Image is the instance (ami etc) we should use Image string `json:"image,omitempty"` @@ -89,7 +89,7 @@ type InstanceGroupSpec struct { MinSize *int32 `json:"minSize,omitempty"` // MaxSize is the maximum size of the pool MaxSize *int32 `json:"maxSize,omitempty"` - // Autoscale determines if autoscaling will be enabled for the group if cluster autoscaler is enabled + // Autoscale determines if autoscaling will be enabled for this instance group if cluster autoscaler is enabled Autoscale *bool `json:"autoscale,omitempty"` // MachineType is the instance class MachineType string `json:"machineType,omitempty"` @@ -134,16 +134,16 @@ type InstanceGroupSpec struct { AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"` // CloudLabels defines additional tags or labels on cloud provider resources CloudLabels map[string]string `json:"cloudLabels,omitempty"` - // NodeLabels indicates the kubernetes labels for nodes in this group + // NodeLabels indicates the kubernetes labels for nodes in this instance group NodeLabels map[string]string `json:"nodeLabels,omitempty"` // FileAssets is a collection of file assets for this instance group FileAssets []FileAssetSpec `json:"fileAssets,omitempty"` - // Describes the tenancy of the instance group. Can be either default or dedicated. + // Describes the tenancy of this instance group. Can be either default or dedicated. // Currently only applies to AWS. Tenancy string `json:"tenancy,omitempty"` // Kubelet overrides kubelet config from the ClusterSpec Kubelet *KubeletConfigSpec `json:"kubelet,omitempty"` - // Taints indicates the kubernetes taints for nodes in this group + // Taints indicates the kubernetes taints for nodes in this instance group Taints []string `json:"taints,omitempty"` // MixedInstancesPolicy defined a optional backing of an AWS ASG by a EC2 Fleet (AWS Only) MixedInstancesPolicy *MixedInstancesPolicySpec `json:"mixedInstancesPolicy,omitempty"` @@ -151,7 +151,7 @@ type InstanceGroupSpec struct { AdditionalUserData []UserData `json:"additionalUserData,omitempty"` // SuspendProcesses disables the listed Scaling Policies SuspendProcesses []string `json:"suspendProcesses,omitempty"` - // ExternalLoadBalancers define loadbalancers that should be attached to the instancegroup + // ExternalLoadBalancers define loadbalancers that should be attached to this instance group ExternalLoadBalancers []LoadBalancer `json:"externalLoadBalancers,omitempty"` // DetailedInstanceMonitoring defines if detailed-monitoring is enabled (AWS only) DetailedInstanceMonitoring *bool `json:"detailedInstanceMonitoring,omitempty"` @@ -175,6 +175,7 @@ type InstanceGroupSpec struct { // InstanceMetadata defines the EC2 instance metadata service options (AWS Only) InstanceMetadata *InstanceMetadataOptions `json:"instanceMetadata,omitempty"` // UpdatePolicy determines the policy for applying upgrades automatically. + // If specified, this value overrides a value specified in the Cluster's "spec.updatePolicy" field. // 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 From 2fc68564be758abd1714a9a3d32b2e93ad78ce0c Mon Sep 17 00:00:00 2001 From: "Steven E. Harris" Date: Fri, 5 Mar 2021 09:41:09 -0500 Subject: [PATCH 3/3] Note new field's impact on OS package installation --- upup/pkg/fi/nodeup/nodetasks/package.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/upup/pkg/fi/nodeup/nodetasks/package.go b/upup/pkg/fi/nodeup/nodetasks/package.go index f00a4f6712505..928467333cc9e 100644 --- a/upup/pkg/fi/nodeup/nodetasks/package.go +++ b/upup/pkg/fi/nodeup/nodetasks/package.go @@ -198,6 +198,9 @@ func (e *Package) findDpkg(c *fi.Context) (*Package, error) { } } + // TODO: Take InstanceGroup-level overriding of the Cluster-level update policy into account + // here. Doing so requires that we make the current InstanceGroup available within Package's + // methods. if fi.StringValue(c.Cluster.Spec.UpdatePolicy) != kops.UpdatePolicyExternal || !installed { return nil, nil } @@ -246,6 +249,9 @@ func (e *Package) findYum(c *fi.Context) (*Package, error) { healthy = fi.Bool(true) } + // TODO: Take InstanceGroup-level overriding of the Cluster-level update policy into account + // here. Doing so requires that we make the current InstanceGroup available within Package's + // methods. if fi.StringValue(c.Cluster.Spec.UpdatePolicy) != kops.UpdatePolicyExternal || !installed { return nil, nil }