Skip to content

Commit

Permalink
Merge pull request #10808 from sbueringer/pr-map-keys
Browse files Browse the repository at this point in the history 
✨ Add map key for MD/MP class & topology in ClusterClass & Cluster.spec.topology
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jun 28, 2024
2 parents 8ce866c + 8163aae commit 41c38f2
Show file tree
Hide file tree
Showing 9 changed files with 92 additions and 40 deletions.
4 changes: 4 additions & 0 deletions api/v1beta1/cluster_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,14 @@ type ControlPlaneTopology struct {
type WorkersTopology struct {
// MachineDeployments is a list of machine deployments in the cluster.
// +optional
// +listType=map
// +listMapKey=name
MachineDeployments []MachineDeploymentTopology `json:"machineDeployments,omitempty"`

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

Expand Down
4 changes: 4 additions & 0 deletions api/v1beta1/clusterclass_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,15 @@ type WorkersClass struct {
// MachineDeployments is a list of machine deployment classes that can be used to create
// a set of worker nodes.
// +optional
// +listType=map
// +listMapKey=class
MachineDeployments []MachineDeploymentClass `json:"machineDeployments,omitempty"`

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

Expand Down
32 changes: 32 additions & 0 deletions api/v1beta1/zz_generated.openapi.go
View file

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

6 changes: 6 additions & 0 deletions config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml
View file

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

6 changes: 6 additions & 0 deletions config/crd/bases/cluster.x-k8s.io_clusters.yaml
View file

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

16 changes: 8 additions & 8 deletions internal/topology/variables/cluster_variable_validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2354,7 +2354,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
name: "Error if value has no definition.",
wantErrs: []validationMatch{
invalid("Invalid value: \"\\\"us-east-1\\\"\": no definitions found for variable \"location\"",
"spec.topology.workers.machineDeployments[0].variables.overrides[location]"),
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[location]"),
},
definitions: []clusterv1.ClusterClassStatusVariable{},
values: []clusterv1.ClusterVariable{
Expand All @@ -2371,7 +2371,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
name: "Fail if value DefinitionFrom field does not match any definition.",
wantErrs: []validationMatch{
invalid("Invalid value: \"1\": no definitions found for variable \"cpu\" from \"non-existent-patch\"",
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu]"),
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu]"),
},
definitions: []clusterv1.ClusterClassStatusVariable{
{
Expand Down Expand Up @@ -2403,9 +2403,9 @@ func Test_ValidateMachineVariables(t *testing.T) {
name: "Fail when values invalid by their definition schema.",
wantErrs: []validationMatch{
invalidType("Invalid value: \"1\": must be of type string: \"integer\"",
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"),
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"),
invalidType("Invalid value: \"\\\"one\\\"\": must be of type integer: \"string\"",
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"),
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"),
},
definitions: []clusterv1.ClusterClassStatusVariable{
{
Expand Down Expand Up @@ -2483,7 +2483,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
name: "Error if integer is above maximum via with CEL expression",
wantErrs: []validationMatch{
invalid("Invalid value: \"99\": failed rule: self <= 1",
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"),
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"),
},
definitions: []clusterv1.ClusterClassStatusVariable{
{
Expand Down Expand Up @@ -2517,7 +2517,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
name: "Error if integer is below minimum via CEL expression",
wantErrs: []validationMatch{
invalid("Invalid value: \"0\": failed rule: self >= 1",
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"),
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"),
},
definitions: []clusterv1.ClusterClassStatusVariable{
{
Expand Down Expand Up @@ -2619,7 +2619,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
name: "Error if integer is not greater than old value via CEL expression",
wantErrs: []validationMatch{
invalid("Invalid value: \"0\": failed rule: self > oldSelf",
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"),
"spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"),
},
definitions: []clusterv1.ClusterClassStatusVariable{
{
Expand Down Expand Up @@ -2661,7 +2661,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotErrs := ValidateMachineVariables(ctx, tt.values, tt.oldValues, tt.definitions,
field.NewPath("spec", "topology", "workers", "machineDeployments").Index(0).Child("variables", "overrides"))
field.NewPath("spec", "topology", "workers", "machineDeployments").Key("mdTopologyName").Child("variables", "overrides"))

checkErrors(t, tt.wantErrs, gotErrs)
})
Expand Down
36 changes: 18 additions & 18 deletions internal/webhooks/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ func validateMachineHealthChecks(cluster *clusterv1.Cluster, clusterClass *clust
for i := range cluster.Spec.Topology.Workers.MachineDeployments {
md := cluster.Spec.Topology.Workers.MachineDeployments[i]
if md.MachineHealthCheck != nil {
fldPath := field.NewPath("spec", "topology", "workers", "machineDeployments", "machineHealthCheck").Index(i)
fldPath := field.NewPath("spec", "topology", "workers", "machineDeployments").Key(md.Name).Child("machineHealthCheck")

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

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

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

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

// Default MachinePool variable overrides.
for i, mp := range cluster.Spec.Topology.Workers.MachinePools {
for _, mp := range cluster.Spec.Topology.Workers.MachinePools {
// Continue if there are no variable overrides.
if mp.Variables == nil || len(mp.Variables.Overrides) == 0 {
continue
}
defaultedVariables, errs := variables.DefaultMachineVariables(mp.Variables.Overrides, clusterClass.Status.Variables,
field.NewPath("spec", "topology", "workers", "machinePools").Index(i).Child("variables", "overrides"))
field.NewPath("spec", "topology", "workers", "machinePools").Key(mp.Name).Child("variables", "overrides"))
if len(errs) > 0 {
allErrs = append(allErrs, errs...)
} else {
Expand Down Expand Up @@ -978,14 +978,14 @@ func validateTopologyMetadata(topology *clusterv1.Topology, fldPath *field.Path)
var allErrs field.ErrorList
allErrs = append(allErrs, topology.ControlPlane.Metadata.Validate(fldPath.Child("controlPlane", "metadata"))...)
if topology.Workers != nil {
for idx, md := range topology.Workers.MachineDeployments {
for _, md := range topology.Workers.MachineDeployments {
allErrs = append(allErrs, md.Metadata.Validate(
fldPath.Child("workers", "machineDeployments").Index(idx).Child("metadata"),
fldPath.Child("workers", "machineDeployments").Key(md.Name).Child("metadata"),
)...)
}
for idx, mp := range topology.Workers.MachinePools {
for _, mp := range topology.Workers.MachinePools {
allErrs = append(allErrs, mp.Metadata.Validate(
fldPath.Child("workers", "machinePools").Index(idx).Child("metadata"),
fldPath.Child("workers", "machinePools").Key(mp.Name).Child("metadata"),
)...)
}
}
Expand All @@ -1003,7 +1003,7 @@ func validateAutoscalerAnnotationsForCluster(cluster *clusterv1.Cluster, cluster
}

fldPath := field.NewPath("spec", "topology")
for i, mdt := range cluster.Spec.Topology.Workers.MachineDeployments {
for _, mdt := range cluster.Spec.Topology.Workers.MachineDeployments {
if mdt.Replicas == nil {
continue
}
Expand All @@ -1012,8 +1012,8 @@ func validateAutoscalerAnnotationsForCluster(cluster *clusterv1.Cluster, cluster
allErrs = append(
allErrs,
field.Invalid(
fldPath.Child("workers", "machineDeployments").Index(i).Child("replicas"),
cluster.Spec.Topology.Workers.MachineDeployments[i].Replicas,
fldPath.Child("workers", "machineDeployments").Key(mdt.Name).Child("replicas"),
mdt.Replicas,
fmt.Sprintf("cannot be set for cluster %q in namespace %q if the same MachineDeploymentTopology has autoscaler annotations",
cluster.Name, cluster.Namespace),
),
Expand All @@ -1036,8 +1036,8 @@ func validateAutoscalerAnnotationsForCluster(cluster *clusterv1.Cluster, cluster
allErrs = append(
allErrs,
field.Invalid(
fldPath.Child("workers", "machineDeployments").Index(i).Child("replicas"),
cluster.Spec.Topology.Workers.MachineDeployments[i].Replicas,
fldPath.Child("workers", "machineDeployments").Key(mdt.Name).Child("replicas"),
mdt.Replicas,
fmt.Sprintf("cannot be set for cluster %q in namespace %q if the source class %q of this MachineDeploymentTopology has autoscaler annotations",
cluster.Name, cluster.Namespace, mdt.Class),
),
Expand Down
4 changes: 2 additions & 2 deletions internal/webhooks/cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1190,8 +1190,8 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) {
wantErrMessage: "Cluster.cluster.x-k8s.io \"cluster1\" is invalid: [" +
"spec.topology.variables[cpu].value: Invalid value: \"-5\": failed rule: self > oldSelf, " +
"spec.topology.controlPlane.variables.overrides[cpu].value: Invalid value: \"-4\": failed rule: self > oldSelf, " +
"spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value: Invalid value: \"-10\": failed rule: self > oldSelf, " +
"spec.topology.workers.machinePools[0].variables.overrides[cpu].value: Invalid value: \"-11\": failed rule: self > oldSelf]",
"spec.topology.workers.machineDeployments[workers1].variables.overrides[cpu].value: Invalid value: \"-10\": failed rule: self > oldSelf, " +
"spec.topology.workers.machinePools[workers1].variables.overrides[cpu].value: Invalid value: \"-11\": failed rule: self > oldSelf]",
},
}
for _, tt := range tests {
Expand Down
Loading

0 comments on commit 41c38f2

Please sign in to comment.