Skip to content

✨ ROSA: Add additionalTags API field to ROSAMachinePool #4869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ spec:
items:
type: string
type: array
additionalTags:
additionalProperties:
type: string
description: AdditionalTags are user-defined tags to be added on the
underlying EC2 instances associated with this machine pool.
type: object
autoRepair:
default: false
description: AutoRepair specifies whether health checks should be
Expand Down Expand Up @@ -83,14 +89,13 @@ spec:
description: Labels specifies labels for the Kubernetes node objects
type: object
nodeDrainGracePeriod:
description: NodeDrainGracePeriod is grace period for how long Pod
description: "NodeDrainGracePeriod is grace period for how long Pod
Disruption Budget-protected workloads will be respected during upgrades.
After this grace period, any workloads protected by Pod Disruption
Budgets that have not been successfully drained from a node will
be forcibly evicted. The nodeDrainGracePeriod can be defined in
minutes, hours ex; 30m, 10h. The max value can be assigned is 10080m|168h
(1 week).
pattern: ^(([0-9])+[m|h])$
be forcibly evicted. \n Valid values are from 0 to 1 week(10080m|168h)
. 0 or empty value means that the MachinePool can be drained without
any time limitation."
type: string
nodePoolName:
description: NodePoolName specifies the name of the nodepool in Rosa
Expand Down
66 changes: 65 additions & 1 deletion docs/book/src/crd/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -22558,7 +22558,7 @@ Tags
<h3 id="infrastructure.cluster.x-k8s.io/v1beta2.Tags">Tags
(<code>map[string]string</code> alias)</p></h3>
<p>
(<em>Appears on:</em><a href="#infrastructure.cluster.x-k8s.io/v1beta2.AWSClusterSpec">AWSClusterSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.AWSMachineSpec">AWSMachineSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.BuildParams">BuildParams</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.SecurityGroup">SecurityGroup</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.SubnetSpec">SubnetSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.VPCSpec">VPCSpec</a>, <a href="#bootstrap.aws.infrastructure.cluster.x-k8s.io/v1alpha1.AWSIAMRoleSpec">AWSIAMRoleSpec</a>, <a href="#bootstrap.aws.infrastructure.cluster.x-k8s.io/v1alpha1.BootstrapUser">BootstrapUser</a>, <a href="#bootstrap.aws.infrastructure.cluster.x-k8s.io/v1beta1.AWSIAMRoleSpec">AWSIAMRoleSpec</a>, <a href="#bootstrap.aws.infrastructure.cluster.x-k8s.io/v1beta1.BootstrapUser">BootstrapUser</a>, <a href="#controlplane.cluster.x-k8s.io/v1beta1.AWSManagedControlPlaneSpec">AWSManagedControlPlaneSpec</a>, <a href="#controlplane.cluster.x-k8s.io/v1beta1.OIDCIdentityProviderConfig">OIDCIdentityProviderConfig</a>, <a href="#controlplane.cluster.x-k8s.io/v1beta2.AWSManagedControlPlaneSpec">AWSManagedControlPlaneSpec</a>, <a href="#controlplane.cluster.x-k8s.io/v1beta2.OIDCIdentityProviderConfig">OIDCIdentityProviderConfig</a>, <a href="#controlplane.cluster.x-k8s.io/v1beta2.RosaControlPlaneSpec">RosaControlPlaneSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta1.AWSMachinePoolSpec">AWSMachinePoolSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta1.AWSManagedMachinePoolSpec">AWSManagedMachinePoolSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta1.AutoScalingGroup">AutoScalingGroup</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta1.FargateProfileSpec">FargateProfileSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.AWSMachinePoolSpec">AWSMachinePoolSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.AWSManagedMachinePoolSpec">AWSManagedMachinePoolSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.AutoScalingGroup">AutoScalingGroup</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.FargateProfileSpec">FargateProfileSpec</a>)
(<em>Appears on:</em><a href="#infrastructure.cluster.x-k8s.io/v1beta2.AWSClusterSpec">AWSClusterSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.AWSMachineSpec">AWSMachineSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.BuildParams">BuildParams</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.SecurityGroup">SecurityGroup</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.SubnetSpec">SubnetSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.VPCSpec">VPCSpec</a>, <a href="#bootstrap.aws.infrastructure.cluster.x-k8s.io/v1alpha1.AWSIAMRoleSpec">AWSIAMRoleSpec</a>, <a href="#bootstrap.aws.infrastructure.cluster.x-k8s.io/v1alpha1.BootstrapUser">BootstrapUser</a>, <a href="#bootstrap.aws.infrastructure.cluster.x-k8s.io/v1beta1.AWSIAMRoleSpec">AWSIAMRoleSpec</a>, <a href="#bootstrap.aws.infrastructure.cluster.x-k8s.io/v1beta1.BootstrapUser">BootstrapUser</a>, <a href="#controlplane.cluster.x-k8s.io/v1beta1.AWSManagedControlPlaneSpec">AWSManagedControlPlaneSpec</a>, <a href="#controlplane.cluster.x-k8s.io/v1beta1.OIDCIdentityProviderConfig">OIDCIdentityProviderConfig</a>, <a href="#controlplane.cluster.x-k8s.io/v1beta2.AWSManagedControlPlaneSpec">AWSManagedControlPlaneSpec</a>, <a href="#controlplane.cluster.x-k8s.io/v1beta2.OIDCIdentityProviderConfig">OIDCIdentityProviderConfig</a>, <a href="#controlplane.cluster.x-k8s.io/v1beta2.RosaControlPlaneSpec">RosaControlPlaneSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta1.AWSMachinePoolSpec">AWSMachinePoolSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta1.AWSManagedMachinePoolSpec">AWSManagedMachinePoolSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta1.AutoScalingGroup">AutoScalingGroup</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta1.FargateProfileSpec">FargateProfileSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.AWSMachinePoolSpec">AWSMachinePoolSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.AWSManagedMachinePoolSpec">AWSManagedMachinePoolSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.AutoScalingGroup">AutoScalingGroup</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.FargateProfileSpec">FargateProfileSpec</a>, <a href="#infrastructure.cluster.x-k8s.io/v1beta2.RosaMachinePoolSpec">RosaMachinePoolSpec</a>)
</p>
<p>
<p>Tags defines a map of tags.</p>
Expand Down Expand Up @@ -25938,6 +25938,20 @@ map[string]string
</tr>
<tr>
<td>
<code>additionalTags</code><br/>
<em>
<a href="#infrastructure.cluster.x-k8s.io/v1beta2.Tags">
Tags
</a>
</em>
</td>
<td>
<em>(Optional)</em>
<p>AdditionalTags are user-defined tags to be added on the underlying EC2 instances associated with this machine pool.</p>
</td>
</tr>
<tr>
<td>
<code>autoRepair</code><br/>
<em>
bool
Expand Down Expand Up @@ -26013,6 +26027,24 @@ with all node instances of the machine pool.</p>
<p>ProviderIDList contain a ProviderID for each machine instance that&rsquo;s currently managed by this machine pool.</p>
</td>
</tr>
<tr>
<td>
<code>nodeDrainGracePeriod</code><br/>
<em>
<a href="https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#Duration">
Kubernetes meta/v1.Duration
</a>
</em>
</td>
<td>
<em>(Optional)</em>
<p>NodeDrainGracePeriod is grace period for how long Pod Disruption Budget-protected workloads will be
respected during upgrades. After this grace period, any workloads protected by Pod Disruption
Budgets that have not been successfully drained from a node will be forcibly evicted.</p>
<p>Valid values are from 0 to 1 week(10080m|168h) .
0 or empty value means that the MachinePool can be drained without any time limitation.</p>
</td>
</tr>
</table>
</td>
</tr>
Expand Down Expand Up @@ -26233,6 +26265,20 @@ map[string]string
</tr>
<tr>
<td>
<code>additionalTags</code><br/>
<em>
<a href="#infrastructure.cluster.x-k8s.io/v1beta2.Tags">
Tags
</a>
</em>
</td>
<td>
<em>(Optional)</em>
<p>AdditionalTags are user-defined tags to be added on the underlying EC2 instances associated with this machine pool.</p>
</td>
</tr>
<tr>
<td>
<code>autoRepair</code><br/>
<em>
bool
Expand Down Expand Up @@ -26308,6 +26354,24 @@ with all node instances of the machine pool.</p>
<p>ProviderIDList contain a ProviderID for each machine instance that&rsquo;s currently managed by this machine pool.</p>
</td>
</tr>
<tr>
<td>
<code>nodeDrainGracePeriod</code><br/>
<em>
<a href="https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#Duration">
Kubernetes meta/v1.Duration
</a>
</em>
</td>
<td>
<em>(Optional)</em>
<p>NodeDrainGracePeriod is grace period for how long Pod Disruption Budget-protected workloads will be
respected during upgrades. After this grace period, any workloads protected by Pod Disruption
Budgets that have not been successfully drained from a node will be forcibly evicted.</p>
<p>Valid values are from 0 to 1 week(10080m|168h) .
0 or empty value means that the MachinePool can be drained without any time limitation.</p>
</td>
</tr>
</tbody>
</table>
<h3 id="infrastructure.cluster.x-k8s.io/v1beta2.RosaMachinePoolStatus">RosaMachinePoolStatus
Expand Down
16 changes: 12 additions & 4 deletions exp/api/v1beta2/rosamachinepool_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
)

Expand Down Expand Up @@ -58,6 +59,11 @@ type RosaMachinePoolSpec struct {
// +optional
Taints []RosaTaint `json:"taints,omitempty"`

// AdditionalTags are user-defined tags to be added on the underlying EC2 instances associated with this machine pool.
// +immutable
// +optional
AdditionalTags infrav1.Tags `json:"additionalTags,omitempty"`

// AutoRepair specifies whether health checks should be enabled for machines
// in the NodePool. The default is false.
// +kubebuilder:default=false
Expand Down Expand Up @@ -92,11 +98,13 @@ type RosaMachinePoolSpec struct {

// NodeDrainGracePeriod is grace period for how long Pod Disruption Budget-protected workloads will be
// respected during upgrades. After this grace period, any workloads protected by Pod Disruption
// Budgets that have not been successfully drained from a node will be forcibly evicted. The nodeDrainGracePeriod
// can be defined in minutes, hours ex; 30m, 10h. The max value can be assigned is 10080m|168h (1 week).
// +kubebuilder:validation:Pattern="^(([0-9])+[m|h])$"
// Budgets that have not been successfully drained from a node will be forcibly evicted.
//
// Valid values are from 0 to 1 week(10080m|168h) .
// 0 or empty value means that the MachinePool can be drained without any time limitation.
//
// +optional
NodeDrainGracePeriod string `json:"nodeDrainGracePeriod,omitempty"`
NodeDrainGracePeriod *metav1.Duration `json:"nodeDrainGracePeriod,omitempty"`
}

// RosaTaint represents a taint to be applied to a node.
Expand Down
24 changes: 24 additions & 0 deletions exp/api/v1beta2/rosamachinepool_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ func (r *ROSAMachinePool) ValidateCreate() (warnings admission.Warnings, err err
allErrs = append(allErrs, err)
}

if err := r.validateNodeDrainGracePeriod(); err != nil {
allErrs = append(allErrs, err)
}

allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...)

if len(allErrs) == 0 {
return nil, nil
}
Expand All @@ -58,7 +64,12 @@ func (r *ROSAMachinePool) ValidateUpdate(old runtime.Object) (warnings admission
allErrs = append(allErrs, err)
}

if err := r.validateNodeDrainGracePeriod(); err != nil {
allErrs = append(allErrs, err)
}

allErrs = append(allErrs, validateImmutable(oldPool.Spec.AdditionalSecurityGroups, r.Spec.AdditionalSecurityGroups, "additionalSecurityGroups")...)
allErrs = append(allErrs, validateImmutable(oldPool.Spec.AdditionalTags, r.Spec.AdditionalTags, "additionalTags")...)

if len(allErrs) == 0 {
return nil, nil
Expand Down Expand Up @@ -88,6 +99,19 @@ func (r *ROSAMachinePool) validateVersion() *field.Error {
return nil
}

func (r *ROSAMachinePool) validateNodeDrainGracePeriod() *field.Error {
if r.Spec.NodeDrainGracePeriod == nil {
return nil
}

if r.Spec.NodeDrainGracePeriod.Minutes() > 10080 {
return field.Invalid(field.NewPath("spec.nodeDrainGracePeriod"), r.Spec.NodeDrainGracePeriod,
"max supported duration is 1 week (10080m|168h)")
}

return nil
}

func validateImmutable(old, updated interface{}, name string) field.ErrorList {
var allErrs field.ErrorList

Expand Down
13 changes: 13 additions & 0 deletions exp/api/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 22 additions & 17 deletions exp/controllers/rosamachinepool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/tools/record"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -362,6 +363,7 @@ func (r *ROSAMachinePoolReconciler) updateNodePool(machinePoolScope *scope.RosaM
// zero-out fields that shouldn't be part of the update call.
desiredSpec.Version = ""
desiredSpec.AdditionalSecurityGroups = nil
desiredSpec.AdditionalTags = nil

npBuilder := nodePoolBuilder(*desiredSpec, machinePoolScope.MachinePool.Spec)
nodePoolSpec, err := npBuilder.Build()
Expand Down Expand Up @@ -401,18 +403,6 @@ func validateMachinePoolSpec(machinePoolScope *scope.RosaMachinePoolScope) (*str
return &message, nil
}

if machinePoolScope.RosaMachinePool.Spec.NodeDrainGracePeriod != "" {
nodeDrainDuration, err := time.ParseDuration(machinePoolScope.RosaMachinePool.Spec.NodeDrainGracePeriod)
if err != nil {
return nil, err
}
// Check if node grace period duration is > 10080m (168h - 1 week)
if nodeDrainDuration.Minutes() > 10080 {
message := "Max supported NodeDrainGracePeriod duration is 10080m|168h (1 week)"
return &message, nil
}
}

// TODO: add more input validations
return nil, nil
}
Expand Down Expand Up @@ -456,16 +446,17 @@ func nodePoolBuilder(rosaMachinePoolSpec expinfrav1.RosaMachinePoolSpec, machine
if rosaMachinePoolSpec.AdditionalSecurityGroups != nil {
awsNodePool = awsNodePool.AdditionalSecurityGroupIds(rosaMachinePoolSpec.AdditionalSecurityGroups...)
}
if rosaMachinePoolSpec.AdditionalTags != nil {
awsNodePool = awsNodePool.Tags(rosaMachinePoolSpec.AdditionalTags)
}
npBuilder.AWSNodePool(awsNodePool)

if rosaMachinePoolSpec.Version != "" {
npBuilder.Version(cmv1.NewVersion().ID(ocm.CreateVersionID(rosaMachinePoolSpec.Version, ocm.DefaultChannelGroup)))
}

if rosaMachinePoolSpec.NodeDrainGracePeriod != "" {
duration, _ := time.ParseDuration(rosaMachinePoolSpec.NodeDrainGracePeriod)
valueBuilder := cmv1.NewValue()
valueBuilder.Value(duration.Minutes()).Unit("minutes")
if rosaMachinePoolSpec.NodeDrainGracePeriod != nil {
valueBuilder := cmv1.NewValue().Value(rosaMachinePoolSpec.NodeDrainGracePeriod.Minutes()).Unit("minutes")
npBuilder.NodeDrainGracePeriod(valueBuilder)
}

Expand All @@ -479,6 +470,7 @@ func nodePoolToRosaMachinePoolSpec(nodePool *cmv1.NodePool) expinfrav1.RosaMachi
AvailabilityZone: nodePool.AvailabilityZone(),
Subnet: nodePool.Subnet(),
Labels: nodePool.Labels(),
AdditionalTags: nodePool.AWSNodePool().Tags(),
AutoRepair: nodePool.AutoRepair(),
InstanceType: nodePool.AWSNodePool().InstanceType(),
TuningConfigs: nodePool.TuningConfigs(),
Expand All @@ -502,6 +494,11 @@ func nodePoolToRosaMachinePoolSpec(nodePool *cmv1.NodePool) expinfrav1.RosaMachi
}
spec.Taints = rosaTaints
}
if nodePool.NodeDrainGracePeriod() != nil {
spec.NodeDrainGracePeriod = &metav1.Duration{
Duration: time.Minute * time.Duration(nodePool.NodeDrainGracePeriod().Value()),
}
}

return spec
}
Expand Down Expand Up @@ -534,7 +531,7 @@ func (r *ROSAMachinePoolReconciler) reconcileProviderIDList(ctx context.Context,
}

func buildEC2FiltersFromTags(tags map[string]string) []*ec2.Filter {
filters := make([]*ec2.Filter, len(tags))
filters := make([]*ec2.Filter, len(tags)+1)
for key, value := range tags {
filters = append(filters, &ec2.Filter{
Name: ptr.To(fmt.Sprintf("tag:%s", key)),
Expand All @@ -544,6 +541,14 @@ func buildEC2FiltersFromTags(tags map[string]string) []*ec2.Filter {
})
}

// only list instances that are running or just started
filters = append(filters, &ec2.Filter{
Name: ptr.To("instance-state-name"),
Values: aws.StringSlice([]string{
"running", "pending",
}),
})

return filters
}

Expand Down
9 changes: 9 additions & 0 deletions exp/controllers/rosamachinepool_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ package controllers

import (
"testing"
"time"

. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"

infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2"
expclusterv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
)
Expand All @@ -20,6 +23,12 @@ func TestNodePoolToRosaMachinePoolSpec(t *testing.T) {
AutoRepair: true,
InstanceType: "m5.large",
TuningConfigs: []string{"config1"},
NodeDrainGracePeriod: &metav1.Duration{
Duration: time.Minute * 10,
},
AdditionalTags: infrav1.Tags{
"tag1": "value1",
},
}

machinePoolSpec := expclusterv1.MachinePoolSpec{
Expand Down