Skip to content

Commit 41c38f2

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#10808 from sbueringer/pr-map-keys
✨ Add map key for MD/MP class & topology in ClusterClass & Cluster.spec.topology
2 parents 8ce866c + 8163aae commit 41c38f2

File tree

9 files changed

+92
-40
lines changed

9 files changed

+92
-40
lines changed

‎api/v1beta1/cluster_types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,14 @@ type ControlPlaneTopology struct {
153153
type WorkersTopology struct {
154154
// MachineDeployments is a list of machine deployments in the cluster.
155155
// +optional
156+
// +listType=map
157+
// +listMapKey=name
156158
MachineDeployments []MachineDeploymentTopology `json:"machineDeployments,omitempty"`
157159

158160
// MachinePools is a list of machine pools in the cluster.
159161
// +optional
162+
// +listType=map
163+
// +listMapKey=name
160164
MachinePools []MachinePoolTopology `json:"machinePools,omitempty"`
161165
}
162166

‎api/v1beta1/clusterclass_types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,15 @@ type WorkersClass struct {
149149
// MachineDeployments is a list of machine deployment classes that can be used to create
150150
// a set of worker nodes.
151151
// +optional
152+
// +listType=map
153+
// +listMapKey=class
152154
MachineDeployments []MachineDeploymentClass `json:"machineDeployments,omitempty"`
153155

154156
// MachinePools is a list of machine pool classes that can be used to create
155157
// a set of worker nodes.
156158
// +optional
159+
// +listType=map
160+
// +listMapKey=class
157161
MachinePools []MachinePoolClass `json:"machinePools,omitempty"`
158162
}
159163

‎api/v1beta1/zz_generated.openapi.go

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

‎config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml

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

‎config/crd/bases/cluster.x-k8s.io_clusters.yaml

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

‎internal/topology/variables/cluster_variable_validation_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2354,7 +2354,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
23542354
name: "Error if value has no definition.",
23552355
wantErrs: []validationMatch{
23562356
invalid("Invalid value: \"\\\"us-east-1\\\"\": no definitions found for variable \"location\"",
2357-
"spec.topology.workers.machineDeployments[0].variables.overrides[location]"),
2357+
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[location]"),
23582358
},
23592359
definitions: []clusterv1.ClusterClassStatusVariable{},
23602360
values: []clusterv1.ClusterVariable{
@@ -2371,7 +2371,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
23712371
name: "Fail if value DefinitionFrom field does not match any definition.",
23722372
wantErrs: []validationMatch{
23732373
invalid("Invalid value: \"1\": no definitions found for variable \"cpu\" from \"non-existent-patch\"",
2374-
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu]"),
2374+
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu]"),
23752375
},
23762376
definitions: []clusterv1.ClusterClassStatusVariable{
23772377
{
@@ -2403,9 +2403,9 @@ func Test_ValidateMachineVariables(t *testing.T) {
24032403
name: "Fail when values invalid by their definition schema.",
24042404
wantErrs: []validationMatch{
24052405
invalidType("Invalid value: \"1\": must be of type string: \"integer\"",
2406-
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"),
2406+
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"),
24072407
invalidType("Invalid value: \"\\\"one\\\"\": must be of type integer: \"string\"",
2408-
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"),
2408+
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"),
24092409
},
24102410
definitions: []clusterv1.ClusterClassStatusVariable{
24112411
{
@@ -2483,7 +2483,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
24832483
name: "Error if integer is above maximum via with CEL expression",
24842484
wantErrs: []validationMatch{
24852485
invalid("Invalid value: \"99\": failed rule: self <= 1",
2486-
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"),
2486+
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"),
24872487
},
24882488
definitions: []clusterv1.ClusterClassStatusVariable{
24892489
{
@@ -2517,7 +2517,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
25172517
name: "Error if integer is below minimum via CEL expression",
25182518
wantErrs: []validationMatch{
25192519
invalid("Invalid value: \"0\": failed rule: self >= 1",
2520-
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"),
2520+
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"),
25212521
},
25222522
definitions: []clusterv1.ClusterClassStatusVariable{
25232523
{
@@ -2619,7 +2619,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
26192619
name: "Error if integer is not greater than old value via CEL expression",
26202620
wantErrs: []validationMatch{
26212621
invalid("Invalid value: \"0\": failed rule: self > oldSelf",
2622-
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"),
2622+
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"),
26232623
},
26242624
definitions: []clusterv1.ClusterClassStatusVariable{
26252625
{
@@ -2661,7 +2661,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
26612661
for _, tt := range tests {
26622662
t.Run(tt.name, func(t *testing.T) {
26632663
gotErrs := ValidateMachineVariables(ctx, tt.values, tt.oldValues, tt.definitions,
2664-
field.NewPath("spec", "topology", "workers", "machineDeployments").Index(0).Child("variables", "overrides"))
2664+
field.NewPath("spec", "topology", "workers", "machineDeployments").Key("mdTopologyName").Child("variables", "overrides"))
26652665

26662666
checkErrors(t, tt.wantErrs, gotErrs)
26672667
})

‎internal/webhooks/cluster.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ func validateMachineHealthChecks(cluster *clusterv1.Cluster, clusterClass *clust
676676
for i := range cluster.Spec.Topology.Workers.MachineDeployments {
677677
md := cluster.Spec.Topology.Workers.MachineDeployments[i]
678678
if md.MachineHealthCheck != nil {
679-
fldPath := field.NewPath("spec", "topology", "workers", "machineDeployments", "machineHealthCheck").Index(i)
679+
fldPath := field.NewPath("spec", "topology", "workers", "machineDeployments").Key(md.Name).Child("machineHealthCheck")
680680

681681
// Validate the MachineDeployment MachineHealthCheck if defined.
682682
if !md.MachineHealthCheck.MachineHealthCheckClass.IsZero() {
@@ -791,7 +791,7 @@ func DefaultAndValidateVariables(ctx context.Context, cluster, oldCluster *clust
791791

792792
if cluster.Spec.Topology.Workers != nil {
793793
// Validate MachineDeployment variable overrides.
794-
for i, md := range cluster.Spec.Topology.Workers.MachineDeployments {
794+
for _, md := range cluster.Spec.Topology.Workers.MachineDeployments {
795795
// Continue if there are no variable overrides.
796796
if md.Variables == nil || len(md.Variables.Overrides) == 0 {
797797
continue
@@ -801,12 +801,12 @@ func DefaultAndValidateVariables(ctx context.Context, cluster, oldCluster *clust
801801
md.Variables.Overrides,
802802
oldMDVariables[md.Name],
803803
clusterClass.Status.Variables,
804-
field.NewPath("spec", "topology", "workers", "machineDeployments").Index(i).Child("variables", "overrides"))...,
804+
field.NewPath("spec", "topology", "workers", "machineDeployments").Key(md.Name).Child("variables", "overrides"))...,
805805
)
806806
}
807807

808808
// Validate MachinePool variable overrides.
809-
for i, mp := range cluster.Spec.Topology.Workers.MachinePools {
809+
for _, mp := range cluster.Spec.Topology.Workers.MachinePools {
810810
// Continue if there are no variable overrides.
811811
if mp.Variables == nil || len(mp.Variables.Overrides) == 0 {
812812
continue
@@ -816,7 +816,7 @@ func DefaultAndValidateVariables(ctx context.Context, cluster, oldCluster *clust
816816
mp.Variables.Overrides,
817817
oldMPVariables[mp.Name],
818818
clusterClass.Status.Variables,
819-
field.NewPath("spec", "topology", "workers", "machinePools").Index(i).Child("variables", "overrides"))...,
819+
field.NewPath("spec", "topology", "workers", "machinePools").Key(mp.Name).Child("variables", "overrides"))...,
820820
)
821821
}
822822
}
@@ -855,13 +855,13 @@ func DefaultVariables(cluster *clusterv1.Cluster, clusterClass *clusterv1.Cluste
855855

856856
if cluster.Spec.Topology.Workers != nil {
857857
// Default MachineDeployment variable overrides.
858-
for i, md := range cluster.Spec.Topology.Workers.MachineDeployments {
858+
for _, md := range cluster.Spec.Topology.Workers.MachineDeployments {
859859
// Continue if there are no variable overrides.
860860
if md.Variables == nil || len(md.Variables.Overrides) == 0 {
861861
continue
862862
}
863863
defaultedVariables, errs := variables.DefaultMachineVariables(md.Variables.Overrides, clusterClass.Status.Variables,
864-
field.NewPath("spec", "topology", "workers", "machineDeployments").Index(i).Child("variables", "overrides"))
864+
field.NewPath("spec", "topology", "workers", "machineDeployments").Key(md.Name).Child("variables", "overrides"))
865865
if len(errs) > 0 {
866866
allErrs = append(allErrs, errs...)
867867
} else {
@@ -870,13 +870,13 @@ func DefaultVariables(cluster *clusterv1.Cluster, clusterClass *clusterv1.Cluste
870870
}
871871

872872
// Default MachinePool variable overrides.
873-
for i, mp := range cluster.Spec.Topology.Workers.MachinePools {
873+
for _, mp := range cluster.Spec.Topology.Workers.MachinePools {
874874
// Continue if there are no variable overrides.
875875
if mp.Variables == nil || len(mp.Variables.Overrides) == 0 {
876876
continue
877877
}
878878
defaultedVariables, errs := variables.DefaultMachineVariables(mp.Variables.Overrides, clusterClass.Status.Variables,
879-
field.NewPath("spec", "topology", "workers", "machinePools").Index(i).Child("variables", "overrides"))
879+
field.NewPath("spec", "topology", "workers", "machinePools").Key(mp.Name).Child("variables", "overrides"))
880880
if len(errs) > 0 {
881881
allErrs = append(allErrs, errs...)
882882
} else {
@@ -978,14 +978,14 @@ func validateTopologyMetadata(topology *clusterv1.Topology, fldPath *field.Path)
978978
var allErrs field.ErrorList
979979
allErrs = append(allErrs, topology.ControlPlane.Metadata.Validate(fldPath.Child("controlPlane", "metadata"))...)
980980
if topology.Workers != nil {
981-
for idx, md := range topology.Workers.MachineDeployments {
981+
for _, md := range topology.Workers.MachineDeployments {
982982
allErrs = append(allErrs, md.Metadata.Validate(
983-
fldPath.Child("workers", "machineDeployments").Index(idx).Child("metadata"),
983+
fldPath.Child("workers", "machineDeployments").Key(md.Name).Child("metadata"),
984984
)...)
985985
}
986-
for idx, mp := range topology.Workers.MachinePools {
986+
for _, mp := range topology.Workers.MachinePools {
987987
allErrs = append(allErrs, mp.Metadata.Validate(
988-
fldPath.Child("workers", "machinePools").Index(idx).Child("metadata"),
988+
fldPath.Child("workers", "machinePools").Key(mp.Name).Child("metadata"),
989989
)...)
990990
}
991991
}
@@ -1003,7 +1003,7 @@ func validateAutoscalerAnnotationsForCluster(cluster *clusterv1.Cluster, cluster
10031003
}
10041004

10051005
fldPath := field.NewPath("spec", "topology")
1006-
for i, mdt := range cluster.Spec.Topology.Workers.MachineDeployments {
1006+
for _, mdt := range cluster.Spec.Topology.Workers.MachineDeployments {
10071007
if mdt.Replicas == nil {
10081008
continue
10091009
}
@@ -1012,8 +1012,8 @@ func validateAutoscalerAnnotationsForCluster(cluster *clusterv1.Cluster, cluster
10121012
allErrs = append(
10131013
allErrs,
10141014
field.Invalid(
1015-
fldPath.Child("workers", "machineDeployments").Index(i).Child("replicas"),
1016-
cluster.Spec.Topology.Workers.MachineDeployments[i].Replicas,
1015+
fldPath.Child("workers", "machineDeployments").Key(mdt.Name).Child("replicas"),
1016+
mdt.Replicas,
10171017
fmt.Sprintf("cannot be set for cluster %q in namespace %q if the same MachineDeploymentTopology has autoscaler annotations",
10181018
cluster.Name, cluster.Namespace),
10191019
),
@@ -1036,8 +1036,8 @@ func validateAutoscalerAnnotationsForCluster(cluster *clusterv1.Cluster, cluster
10361036
allErrs = append(
10371037
allErrs,
10381038
field.Invalid(
1039-
fldPath.Child("workers", "machineDeployments").Index(i).Child("replicas"),
1040-
cluster.Spec.Topology.Workers.MachineDeployments[i].Replicas,
1039+
fldPath.Child("workers", "machineDeployments").Key(mdt.Name).Child("replicas"),
1040+
mdt.Replicas,
10411041
fmt.Sprintf("cannot be set for cluster %q in namespace %q if the source class %q of this MachineDeploymentTopology has autoscaler annotations",
10421042
cluster.Name, cluster.Namespace, mdt.Class),
10431043
),

‎internal/webhooks/cluster_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,8 +1190,8 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) {
11901190
wantErrMessage: "Cluster.cluster.x-k8s.io \"cluster1\" is invalid: [" +
11911191
"spec.topology.variables[cpu].value: Invalid value: \"-5\": failed rule: self > oldSelf, " +
11921192
"spec.topology.controlPlane.variables.overrides[cpu].value: Invalid value: \"-4\": failed rule: self > oldSelf, " +
1193-
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value: Invalid value: \"-10\": failed rule: self > oldSelf, " +
1194-
"spec.topology.workers.machinePools[0].variables.overrides[cpu].value: Invalid value: \"-11\": failed rule: self > oldSelf]",
1193+
"spec.topology.workers.machineDeployments[workers1].variables.overrides[cpu].value: Invalid value: \"-10\": failed rule: self > oldSelf, " +
1194+
"spec.topology.workers.machinePools[workers1].variables.overrides[cpu].value: Invalid value: \"-11\": failed rule: self > oldSelf]",
11951195
},
11961196
}
11971197
for _, tt := range tests {

0 commit comments

Comments
 (0)