Merge pull request #10913 from seh/scope-os-update-policy-to-instance-group-too

Honor OS update policy at InstanceGroup level too
This commit is contained in:
Kubernetes Prow Robot 2021-03-12 22:03:03 -08:00 committed by GitHub
commit ad7c793050
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 185 additions and 45 deletions

View File

@ -4074,10 +4074,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

View File

@ -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
@ -814,13 +814,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. 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

View File

@ -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
}

View File

@ -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"`

View File

@ -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"`
@ -112,7 +112,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"`
@ -121,7 +121,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"`
@ -135,15 +135,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"`
@ -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"`
@ -174,6 +174,12 @@ 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.
// 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
UpdatePolicy *string `json:"updatePolicy,omitempty"`
}
const (
@ -188,7 +194,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.

View File

@ -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"
)

View File

@ -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"`

View File

@ -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"`
@ -132,16 +132,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"`
@ -149,7 +149,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"`
@ -172,6 +172,12 @@ 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.
// 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
UpdatePolicy *string `json:"updatePolicy,omitempty"`
}
const (
@ -186,7 +192,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.

View File

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

View File

@ -2179,6 +2179,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
}

View File

@ -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
}

View File

@ -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)
})
}
}

View File

@ -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 {

View File

@ -2345,6 +2345,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
}

View File

@ -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) {

View File

@ -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
}