Skip to content

Commit 0e0e474

Browse files
muraeemuraee
committed
add additionalTags API fied to ROSAMachinePool
- use the correct typemetav1.Duration for NodeDrainGracePeriod field - fixed reconcileProviderIDList() listing terminated/stopped instances
1 parent b2bebfb commit 0e0e474

File tree

7 files changed

+155
-27
lines changed

7 files changed

+155
-27
lines changed

‎config/crd/bases/infrastructure.cluster.x-k8s.io_rosamachinepools.yaml‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ spec:
5353
items:
5454
type: string
5555
type: array
56+
additionalTags:
57+
additionalProperties:
58+
type: string
59+
description: AdditionalTags are user-defined tags to be added on the
60+
underlying EC2 instances associated with this machine pool.
61+
type: object
5662
autoRepair:
5763
default: false
5864
description: AutoRepair specifies whether health checks should be
@@ -83,14 +89,13 @@ spec:
8389
description: Labels specifies labels for the Kubernetes node objects
8490
type: object
8591
nodeDrainGracePeriod:
86-
description: NodeDrainGracePeriod is grace period for how long Pod
92+
description: "NodeDrainGracePeriod is grace period for how long Pod
8793
Disruption Budget-protected workloads will be respected during upgrades.
8894
After this grace period, any workloads protected by Pod Disruption
8995
Budgets that have not been successfully drained from a node will
90-
be forcibly evicted. The nodeDrainGracePeriod can be defined in
91-
minutes, hours ex; 30m, 10h. The max value can be assigned is 10080m|168h
92-
(1 week).
93-
pattern: ^(([0-9])+[m|h])$
96+
be forcibly evicted. \n Valid values are from 0 to 1 week(10080m|168h)
97+
. 0 or empty value means that the MachinePool can be drained without
98+
any time limitation."
9499
type: string
95100
nodePoolName:
96101
description: NodePoolName specifies the name of the nodepool in Rosa

‎docs/book/src/crd/index.md‎

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22558,7 +22558,7 @@ Tags
2255822558
<h3 id="infrastructure.cluster.x-k8s.io/v1beta2.Tags">Tags
2255922559
(<code>map[string]string</code> alias)</p></h3>
2256022560
<p>
22561-
(<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>)
22561+
(<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>)
2256222562
</p>
2256322563
<p>
2256422564
<p>Tags defines a map of tags.</p>
@@ -25938,6 +25938,20 @@ map[string]string
2593825938
</tr>
2593925939
<tr>
2594025940
<td>
25941+
<code>additionalTags</code><br/>
25942+
<em>
25943+
<a href="#infrastructure.cluster.x-k8s.io/v1beta2.Tags">
25944+
Tags
25945+
</a>
25946+
</em>
25947+
</td>
25948+
<td>
25949+
<em>(Optional)</em>
25950+
<p>AdditionalTags are user-defined tags to be added on the underlying EC2 instances associated with this machine pool.</p>
25951+
</td>
25952+
</tr>
25953+
<tr>
25954+
<td>
2594125955
<code>autoRepair</code><br/>
2594225956
<em>
2594325957
bool
@@ -26013,6 +26027,24 @@ with all node instances of the machine pool.</p>
2601326027
<p>ProviderIDList contain a ProviderID for each machine instance that&rsquo;s currently managed by this machine pool.</p>
2601426028
</td>
2601526029
</tr>
26030+
<tr>
26031+
<td>
26032+
<code>nodeDrainGracePeriod</code><br/>
26033+
<em>
26034+
<a href="https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#Duration">
26035+
Kubernetes meta/v1.Duration
26036+
</a>
26037+
</em>
26038+
</td>
26039+
<td>
26040+
<em>(Optional)</em>
26041+
<p>NodeDrainGracePeriod is grace period for how long Pod Disruption Budget-protected workloads will be
26042+
respected during upgrades. After this grace period, any workloads protected by Pod Disruption
26043+
Budgets that have not been successfully drained from a node will be forcibly evicted.</p>
26044+
<p>Valid values are from 0 to 1 week(10080m|168h) .
26045+
0 or empty value means that the MachinePool can be drained without any time limitation.</p>
26046+
</td>
26047+
</tr>
2601626048
</table>
2601726049
</td>
2601826050
</tr>
@@ -26233,6 +26265,20 @@ map[string]string
2623326265
</tr>
2623426266
<tr>
2623526267
<td>
26268+
<code>additionalTags</code><br/>
26269+
<em>
26270+
<a href="#infrastructure.cluster.x-k8s.io/v1beta2.Tags">
26271+
Tags
26272+
</a>
26273+
</em>
26274+
</td>
26275+
<td>
26276+
<em>(Optional)</em>
26277+
<p>AdditionalTags are user-defined tags to be added on the underlying EC2 instances associated with this machine pool.</p>
26278+
</td>
26279+
</tr>
26280+
<tr>
26281+
<td>
2623626282
<code>autoRepair</code><br/>
2623726283
<em>
2623826284
bool
@@ -26308,6 +26354,24 @@ with all node instances of the machine pool.</p>
2630826354
<p>ProviderIDList contain a ProviderID for each machine instance that&rsquo;s currently managed by this machine pool.</p>
2630926355
</td>
2631026356
</tr>
26357+
<tr>
26358+
<td>
26359+
<code>nodeDrainGracePeriod</code><br/>
26360+
<em>
26361+
<a href="https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#Duration">
26362+
Kubernetes meta/v1.Duration
26363+
</a>
26364+
</em>
26365+
</td>
26366+
<td>
26367+
<em>(Optional)</em>
26368+
<p>NodeDrainGracePeriod is grace period for how long Pod Disruption Budget-protected workloads will be
26369+
respected during upgrades. After this grace period, any workloads protected by Pod Disruption
26370+
Budgets that have not been successfully drained from a node will be forcibly evicted.</p>
26371+
<p>Valid values are from 0 to 1 week(10080m|168h) .
26372+
0 or empty value means that the MachinePool can be drained without any time limitation.</p>
26373+
</td>
26374+
</tr>
2631126375
</tbody>
2631226376
</table>
2631326377
<h3 id="infrastructure.cluster.x-k8s.io/v1beta2.RosaMachinePoolStatus">RosaMachinePoolStatus

‎exp/api/v1beta2/rosamachinepool_types.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
corev1 "k8s.io/api/core/v1"
2121
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2222

23+
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
2324
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
2425
)
2526

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

62+
// AdditionalTags are user-defined tags to be added on the underlying EC2 instances associated with this machine pool.
63+
// +immutable
64+
// +optional
65+
AdditionalTags infrav1.Tags `json:"additionalTags,omitempty"`
66+
6167
// AutoRepair specifies whether health checks should be enabled for machines
6268
// in the NodePool. The default is false.
6369
// +kubebuilder:default=false
@@ -92,11 +98,13 @@ type RosaMachinePoolSpec struct {
9298

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

102110
// RosaTaint represents a taint to be applied to a node.

‎exp/api/v1beta2/rosamachinepool_webhook.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ func (r *ROSAMachinePool) ValidateCreate() (warnings admission.Warnings, err err
3333
allErrs = append(allErrs, err)
3434
}
3535

36+
if err := r.validateNodeDrainGracePeriod(); err != nil {
37+
allErrs = append(allErrs, err)
38+
}
39+
40+
allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...)
41+
3642
if len(allErrs) == 0 {
3743
return nil, nil
3844
}
@@ -58,7 +64,12 @@ func (r *ROSAMachinePool) ValidateUpdate(old runtime.Object) (warnings admission
5864
allErrs = append(allErrs, err)
5965
}
6066

67+
if err := r.validateNodeDrainGracePeriod(); err != nil {
68+
allErrs = append(allErrs, err)
69+
}
70+
6171
allErrs = append(allErrs, validateImmutable(oldPool.Spec.AdditionalSecurityGroups, r.Spec.AdditionalSecurityGroups, "additionalSecurityGroups")...)
72+
allErrs = append(allErrs, validateImmutable(oldPool.Spec.AdditionalTags, r.Spec.AdditionalTags, "additionalTags")...)
6273

6374
if len(allErrs) == 0 {
6475
return nil, nil
@@ -88,6 +99,19 @@ func (r *ROSAMachinePool) validateVersion() *field.Error {
8899
return nil
89100
}
90101

102+
func (r *ROSAMachinePool) validateNodeDrainGracePeriod() *field.Error {
103+
if r.Spec.NodeDrainGracePeriod == nil {
104+
return nil
105+
}
106+
107+
if r.Spec.NodeDrainGracePeriod.Minutes() > 10080 {
108+
return field.Invalid(field.NewPath("spec.nodeDrainGracePeriod"), r.Spec.NodeDrainGracePeriod,
109+
"max supported duration is 1 week (10080m|168h)")
110+
}
111+
112+
return nil
113+
}
114+
91115
func validateImmutable(old, updated interface{}, name string) field.ErrorList {
92116
var allErrs field.ErrorList
93117

‎exp/api/v1beta2/zz_generated.deepcopy.go‎

Lines changed: 13 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎exp/controllers/rosamachinepool_controller.go‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/pkg/errors"
1515
corev1 "k8s.io/api/core/v1"
1616
apierrors "k8s.io/apimachinery/pkg/api/errors"
17+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1718
"k8s.io/apimachinery/pkg/runtime/schema"
1819
"k8s.io/client-go/tools/record"
1920
"k8s.io/klog/v2"
@@ -362,6 +363,7 @@ func (r *ROSAMachinePoolReconciler) updateNodePool(machinePoolScope *scope.RosaM
362363
// zero-out fields that shouldn't be part of the update call.
363364
desiredSpec.Version = ""
364365
desiredSpec.AdditionalSecurityGroups = nil
366+
desiredSpec.AdditionalTags = nil
365367

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

404-
if machinePoolScope.RosaMachinePool.Spec.NodeDrainGracePeriod != "" {
405-
nodeDrainDuration, err := time.ParseDuration(machinePoolScope.RosaMachinePool.Spec.NodeDrainGracePeriod)
406-
if err != nil {
407-
return nil, err
408-
}
409-
// Check if node grace period duration is > 10080m (168h - 1 week)
410-
if nodeDrainDuration.Minutes() > 10080 {
411-
message := "Max supported NodeDrainGracePeriod duration is 10080m|168h (1 week)"
412-
return &message, nil
413-
}
414-
}
415-
416406
// TODO: add more input validations
417407
return nil, nil
418408
}
@@ -456,16 +446,17 @@ func nodePoolBuilder(rosaMachinePoolSpec expinfrav1.RosaMachinePoolSpec, machine
456446
if rosaMachinePoolSpec.AdditionalSecurityGroups != nil {
457447
awsNodePool = awsNodePool.AdditionalSecurityGroupIds(rosaMachinePoolSpec.AdditionalSecurityGroups...)
458448
}
449+
if rosaMachinePoolSpec.AdditionalTags != nil {
450+
awsNodePool = awsNodePool.Tags(rosaMachinePoolSpec.AdditionalTags)
451+
}
459452
npBuilder.AWSNodePool(awsNodePool)
460453

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

465-
if rosaMachinePoolSpec.NodeDrainGracePeriod != "" {
466-
duration, _ := time.ParseDuration(rosaMachinePoolSpec.NodeDrainGracePeriod)
467-
valueBuilder := cmv1.NewValue()
468-
valueBuilder.Value(duration.Minutes()).Unit("minutes")
458+
if rosaMachinePoolSpec.NodeDrainGracePeriod != nil {
459+
valueBuilder := cmv1.NewValue().Value(rosaMachinePoolSpec.NodeDrainGracePeriod.Minutes()).Unit("minutes")
469460
npBuilder.NodeDrainGracePeriod(valueBuilder)
470461
}
471462

@@ -479,6 +470,7 @@ func nodePoolToRosaMachinePoolSpec(nodePool *cmv1.NodePool) expinfrav1.RosaMachi
479470
AvailabilityZone: nodePool.AvailabilityZone(),
480471
Subnet: nodePool.Subnet(),
481472
Labels: nodePool.Labels(),
473+
AdditionalTags: nodePool.AWSNodePool().Tags(),
482474
AutoRepair: nodePool.AutoRepair(),
483475
InstanceType: nodePool.AWSNodePool().InstanceType(),
484476
TuningConfigs: nodePool.TuningConfigs(),
@@ -502,6 +494,11 @@ func nodePoolToRosaMachinePoolSpec(nodePool *cmv1.NodePool) expinfrav1.RosaMachi
502494
}
503495
spec.Taints = rosaTaints
504496
}
497+
if nodePool.NodeDrainGracePeriod() != nil {
498+
spec.NodeDrainGracePeriod = &metav1.Duration{
499+
Duration: time.Minute * time.Duration(nodePool.NodeDrainGracePeriod().Value()),
500+
}
501+
}
505502

506503
return spec
507504
}
@@ -534,7 +531,7 @@ func (r *ROSAMachinePoolReconciler) reconcileProviderIDList(ctx context.Context,
534531
}
535532

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

544+
// only list instances that are running or just started
545+
filters = append(filters, &ec2.Filter{
546+
Name: ptr.To("instance-state-name"),
547+
Values: aws.StringSlice([]string{
548+
"running", "pending",
549+
}),
550+
})
551+
547552
return filters
548553
}
549554

‎exp/controllers/rosamachinepool_controller_test.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ package controllers
22

33
import (
44
"testing"
5+
"time"
56

67
. "github.com/onsi/gomega"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
79
"k8s.io/utils/ptr"
810

11+
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
912
expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2"
1013
expclusterv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
1114
)
@@ -20,6 +23,12 @@ func TestNodePoolToRosaMachinePoolSpec(t *testing.T) {
2023
AutoRepair: true,
2124
InstanceType: "m5.large",
2225
TuningConfigs: []string{"config1"},
26+
NodeDrainGracePeriod: &metav1.Duration{
27+
Duration: time.Minute * 10,
28+
},
29+
AdditionalTags: infrav1.Tags{
30+
"tag1": "value1",
31+
},
2332
}
2433

2534
machinePoolSpec := expclusterv1.MachinePoolSpec{

0 commit comments

Comments
 (0)