metropolis: remove stutter in ClusterConfiguration.KubernetesConfig
We already know this is a config (it lives in ClusterConfiguration), no
need to call that a config again.
This doesn't break any compatibility yet as field names are not (yet)
under a stability guarantee.
Change-Id: Ib6492d1c8303cbd0620b979b8047ec9757e301c0
Reviewed-on: https://review.monogon.dev/c/monogon/+/3594
Reviewed-by: Lorenz Brun <lorenz@monogon.tech>
Tested-by: Jenkins CI
diff --git a/metropolis/node/core/curator/reconfigure.go b/metropolis/node/core/curator/reconfigure.go
index 705aa41..8349a68 100644
--- a/metropolis/node/core/curator/reconfigure.go
+++ b/metropolis/node/core/curator/reconfigure.go
@@ -42,7 +42,7 @@
merged := proto.Clone(existing).(*cpb.ClusterConfiguration)
for _, path := range mask.Paths {
- handled, err := reconfigureKubernetesConfig(base, new, existing, merged, path)
+ handled, err := reconfigureKubernetes(base, new, existing, merged, path)
if err != nil {
return nil, err
}
@@ -54,42 +54,42 @@
return merged, nil
}
-// reconfigureKubernetesConfig does a three-way merge of Kubernetes configuration
+// reconfigureKubernetes does a three-way merge of Kubernetes configuration
// (new, existing and optional base) of a given protobuf field path into merged.
//
// An error is returned if there is an issue applying the given change.
// Otherwise, a boolean value is returned, indicating whether this given filed
// path was handled.
-func reconfigureKubernetesConfig(base, new, existing, merged *cpb.ClusterConfiguration, path string) (bool, error) {
- if path == "kubernetes_config" {
- return false, status.Error(codes.InvalidArgument, "cannot mutate kubernetes_config directly, only subfields")
+func reconfigureKubernetes(base, new, existing, merged *cpb.ClusterConfiguration, path string) (bool, error) {
+ if path == "kubernetes" {
+ return false, status.Error(codes.InvalidArgument, "cannot mutate kubernetes directly, only subfields")
}
- if !strings.HasPrefix(path, "kubernetes_config.") {
+ if !strings.HasPrefix(path, "kubernetes.") {
return false, nil
}
- // KubernetesConfig should always exist in stored configs.
- if merged.KubernetesConfig == nil {
- merged.KubernetesConfig = &cpb.ClusterConfiguration_KubernetesConfig{}
+ // Kubernetes should always exist in stored configs.
+ if merged.Kubernetes == nil {
+ merged.Kubernetes = &cpb.ClusterConfiguration_Kubernetes{}
}
- if existing.KubernetesConfig == nil {
- existing.KubernetesConfig = &cpb.ClusterConfiguration_KubernetesConfig{}
+ if existing.Kubernetes == nil {
+ existing.Kubernetes = &cpb.ClusterConfiguration_Kubernetes{}
}
- // Check KubernetesConfig in user structs.
- if new.KubernetesConfig == nil {
+ // Check Kubernetes in user structs.
+ if new.Kubernetes == nil {
return false, status.Errorf(codes.InvalidArgument, "cannot reference field %s in new_config", path)
}
- if base != nil && base.KubernetesConfig == nil {
+ if base != nil && base.Kubernetes == nil {
return false, status.Errorf(codes.InvalidArgument, "cannot reference field %s in old_config", path)
}
switch path {
- case "kubernetes_config.node_labels_to_synchronize":
- if base != nil && cmp.Diff(base.KubernetesConfig.NodeLabelsToSynchronize, existing.KubernetesConfig.NodeLabelsToSynchronize, protocmp.Transform()) != "" {
- return false, status.Error(codes.FailedPrecondition, "base_config.kubernetes_config.node_labels_to_synchronize different from current value")
+ case "kubernetes.node_labels_to_synchronize":
+ if base != nil && cmp.Diff(base.Kubernetes.NodeLabelsToSynchronize, existing.Kubernetes.NodeLabelsToSynchronize, protocmp.Transform()) != "" {
+ return false, status.Error(codes.FailedPrecondition, "base_config.kubernetes.node_labels_to_synchronize different from current value")
}
- merged.KubernetesConfig.NodeLabelsToSynchronize = new.KubernetesConfig.NodeLabelsToSynchronize
+ merged.Kubernetes.NodeLabelsToSynchronize = new.Kubernetes.NodeLabelsToSynchronize
default:
return false, status.Errorf(codes.InvalidArgument, "cannot mutate %s", path)
}
diff --git a/metropolis/node/core/curator/reconfigure_test.go b/metropolis/node/core/curator/reconfigure_test.go
index 41000a4..fb363da 100644
--- a/metropolis/node/core/curator/reconfigure_test.go
+++ b/metropolis/node/core/curator/reconfigure_test.go
@@ -15,10 +15,10 @@
res := &cpb.ClusterConfiguration{
TpmMode: cpb.ClusterConfiguration_TPM_MODE_BEST_EFFORT,
StorageSecurityPolicy: cpb.ClusterConfiguration_STORAGE_SECURITY_POLICY_PERMISSIVE,
- KubernetesConfig: &cpb.ClusterConfiguration_KubernetesConfig{},
+ Kubernetes: &cpb.ClusterConfiguration_Kubernetes{},
}
for _, regex := range regexes {
- res.KubernetesConfig.NodeLabelsToSynchronize = append(res.KubernetesConfig.NodeLabelsToSynchronize, &cpb.ClusterConfiguration_KubernetesConfig_NodeLabelsToSynchronize{
+ res.Kubernetes.NodeLabelsToSynchronize = append(res.Kubernetes.NodeLabelsToSynchronize, &cpb.ClusterConfiguration_Kubernetes_NodeLabelsToSynchronize{
Regexp: regex,
})
}
@@ -59,34 +59,34 @@
// Case 3: reconfigure kubernetes node labels.
{
base: &cpb.ClusterConfiguration{
- KubernetesConfig: &cpb.ClusterConfiguration_KubernetesConfig{
- NodeLabelsToSynchronize: []*cpb.ClusterConfiguration_KubernetesConfig_NodeLabelsToSynchronize{
+ Kubernetes: &cpb.ClusterConfiguration_Kubernetes{
+ NodeLabelsToSynchronize: []*cpb.ClusterConfiguration_Kubernetes_NodeLabelsToSynchronize{
{Regexp: "^foo$"},
},
},
},
new: &cpb.ClusterConfiguration{
- KubernetesConfig: &cpb.ClusterConfiguration_KubernetesConfig{
- NodeLabelsToSynchronize: []*cpb.ClusterConfiguration_KubernetesConfig_NodeLabelsToSynchronize{
+ Kubernetes: &cpb.ClusterConfiguration_Kubernetes{
+ NodeLabelsToSynchronize: []*cpb.ClusterConfiguration_Kubernetes_NodeLabelsToSynchronize{
{Regexp: "^bar$"},
},
},
},
existing: mkCfg("^foo$"),
- mask: &fieldmaskpb.FieldMask{Paths: []string{"kubernetes_config.node_labels_to_synchronize"}},
+ mask: &fieldmaskpb.FieldMask{Paths: []string{"kubernetes.node_labels_to_synchronize"}},
result: mkCfg("^bar$"),
},
// Case 4: reconfigure kubernetes node labels without base.
{
new: &cpb.ClusterConfiguration{
- KubernetesConfig: &cpb.ClusterConfiguration_KubernetesConfig{
- NodeLabelsToSynchronize: []*cpb.ClusterConfiguration_KubernetesConfig_NodeLabelsToSynchronize{
+ Kubernetes: &cpb.ClusterConfiguration_Kubernetes{
+ NodeLabelsToSynchronize: []*cpb.ClusterConfiguration_Kubernetes_NodeLabelsToSynchronize{
{Regexp: "^bar$"},
},
},
},
existing: mkCfg("^foo$"),
- mask: &fieldmaskpb.FieldMask{Paths: []string{"kubernetes_config.node_labels_to_synchronize"}},
+ mask: &fieldmaskpb.FieldMask{Paths: []string{"kubernetes.node_labels_to_synchronize"}},
result: mkCfg("^bar$"),
},
// Case 5: no-op with an empty base.
@@ -124,7 +124,7 @@
{
new: &cpb.ClusterConfiguration{},
existing: &cpb.ClusterConfiguration{},
- mask: &fieldmaskpb.FieldMask{Paths: []string{"kubernetes_config.node_labels_to_synchronize"}},
+ mask: &fieldmaskpb.FieldMask{Paths: []string{"kubernetes.node_labels_to_synchronize"}},
result: &cpb.ClusterConfiguration{},
shouldFail: true,
},
@@ -132,12 +132,12 @@
{
base: &cpb.ClusterConfiguration{},
new: &cpb.ClusterConfiguration{
- KubernetesConfig: &cpb.ClusterConfiguration_KubernetesConfig{
- NodeLabelsToSynchronize: []*cpb.ClusterConfiguration_KubernetesConfig_NodeLabelsToSynchronize{},
+ Kubernetes: &cpb.ClusterConfiguration_Kubernetes{
+ NodeLabelsToSynchronize: []*cpb.ClusterConfiguration_Kubernetes_NodeLabelsToSynchronize{},
},
},
existing: &cpb.ClusterConfiguration{},
- mask: &fieldmaskpb.FieldMask{Paths: []string{"kubernetes_config.node_labels_to_synchronize"}},
+ mask: &fieldmaskpb.FieldMask{Paths: []string{"kubernetes.node_labels_to_synchronize"}},
result: &cpb.ClusterConfiguration{},
shouldFail: true,
},
diff --git a/metropolis/node/core/curator/state_cluster.go b/metropolis/node/core/curator/state_cluster.go
index d5b6001..c645acf 100644
--- a/metropolis/node/core/curator/state_cluster.go
+++ b/metropolis/node/core/curator/state_cluster.go
@@ -25,7 +25,7 @@
ClusterDomain string
TPMMode cpb.ClusterConfiguration_TPMMode
StorageSecurityPolicy cpb.ClusterConfiguration_StorageSecurityPolicy
- NodeLabelsToSynchronizeToKubernetes []*cpb.ClusterConfiguration_KubernetesConfig_NodeLabelsToSynchronize
+ NodeLabelsToSynchronizeToKubernetes []*cpb.ClusterConfiguration_Kubernetes_NodeLabelsToSynchronize
}
// DefaultClusterConfiguration is the default cluster configuration for a newly
@@ -178,7 +178,7 @@
TPMMode: cc.TpmMode,
StorageSecurityPolicy: cc.StorageSecurityPolicy,
}
- if kc := cc.KubernetesConfig; kc != nil {
+ if kc := cc.Kubernetes; kc != nil {
c.NodeLabelsToSynchronizeToKubernetes = kc.NodeLabelsToSynchronize
}
@@ -207,7 +207,7 @@
ClusterDomain: c.ClusterDomain,
TpmMode: c.TPMMode,
StorageSecurityPolicy: c.StorageSecurityPolicy,
- KubernetesConfig: &cpb.ClusterConfiguration_KubernetesConfig{
+ Kubernetes: &cpb.ClusterConfiguration_Kubernetes{
NodeLabelsToSynchronize: c.NodeLabelsToSynchronizeToKubernetes,
},
}, nil
diff --git a/metropolis/node/kubernetes/labelmaker.go b/metropolis/node/kubernetes/labelmaker.go
index 1713223..8cd513f 100644
--- a/metropolis/node/kubernetes/labelmaker.go
+++ b/metropolis/node/kubernetes/labelmaker.go
@@ -127,8 +127,8 @@
if err != nil {
return fmt.Errorf("could not get cluster info: %w", err)
}
- if info.ClusterConfiguration != nil && info.ClusterConfiguration.KubernetesConfig != nil {
- for _, rule := range info.ClusterConfiguration.KubernetesConfig.NodeLabelsToSynchronize {
+ if info.ClusterConfiguration != nil && info.ClusterConfiguration.Kubernetes != nil {
+ for _, rule := range info.ClusterConfiguration.Kubernetes.NodeLabelsToSynchronize {
if rule.Regexp != "" {
re, err := regexp.Compile(rule.Regexp)
if err != nil {