Put versioned API of cluster into state store

kubernetes · Jun 12, 2021 · 30faa3a · 30faa3a
1 parent cfc93e5
commit 30faa3a
Show file tree

Hide file tree

Showing 20 changed files with 91 additions and 164 deletions.
diff --git a/cmd/kops-controller/controllers/BUILD.bazel b/cmd/kops-controller/controllers/BUILD.bazel
diff --git a/cmd/kops-controller/controllers/legacy_node_controller.go b/cmd/kops-controller/controllers/legacy_node_controller.go
@@ -28,6 +28,7 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/apis/kops/registry"
+	"k8s.io/kops/pkg/kopscodecs"
 	"k8s.io/kops/pkg/nodeidentity"
 	"k8s.io/kops/pkg/nodelabels"
 	"k8s.io/kops/upup/pkg/fi/utils"
@@ -205,12 +206,14 @@ func (r *LegacyNodeReconciler) loadCluster(p vfs.Path) (*kops.Cluster, error) {
 		return nil, fmt.Errorf("error loading Cluster %q: %v", p, err)
 	}
 
-	cluster := &kops.Cluster{}
-	if err := utils.YamlUnmarshal(b, cluster); err != nil {
+	o, _, err := kopscodecs.Decode(b, nil)
+	if err != nil {
 		return nil, fmt.Errorf("error parsing Cluster %q: %v", p, err)
 	}
-
-	return cluster, nil
+	if cluster, ok := o.(*kops.Cluster); ok {
+		return cluster, nil
+	}
+	return nil, fmt.Errorf("unexpected object type for Cluster %q: %T", p, o)
 }
 
 // loadInstanceGroup loads a kops.InstanceGroup object from the vfs backing store

diff --git a/cmd/kops/create_cluster.go b/cmd/kops/create_cluster.go
@@ -591,15 +591,6 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
 		return fmt.Errorf("error writing updated configuration: %v", err)
 	}
 
-	configBase, err := clientset.ConfigBaseFor(cluster)
-	if err != nil {
-		return fmt.Errorf("error building ConfigBase for cluster: %v", err)
-	}
-	err = registry.WriteConfigDeprecated(cluster, configBase.Join(registry.PathClusterCompleted), fullCluster)
-	if err != nil {
-		return fmt.Errorf("error writing completed cluster spec: %v", err)
-	}
-
 	if len(c.SSHPublicKeys) == 0 {
 		autoloadSSHPublicKeys := true
 		switch c.CloudProvider {

diff --git a/cmd/kops/edit_cluster.go b/cmd/kops/edit_cluster.go
@@ -29,7 +29,6 @@ import (
 	"github.com/spf13/cobra"
 	"k8s.io/kops/cmd/kops/util"
 	api "k8s.io/kops/pkg/apis/kops"
-	"k8s.io/kops/pkg/apis/kops/registry"
 	"k8s.io/kops/pkg/apis/kops/validation"
 	"k8s.io/kops/pkg/assets"
 	"k8s.io/kops/pkg/commands"
@@ -237,11 +236,6 @@ func RunEditCluster(ctx context.Context, f *util.Factory, cmd *cobra.Command, ar
 			continue
 		}
 
-		configBase, err := registry.ConfigBase(newCluster)
-		if err != nil {
-			return preservedFile(err, file, out)
-		}
-
 		// Retrieve the current status of the cluster.  This will eventually be part of the cluster object.
 		status, err := cloud.FindClusterStatus(oldCluster)
 		if err != nil {
@@ -254,11 +248,6 @@ func RunEditCluster(ctx context.Context, f *util.Factory, cmd *cobra.Command, ar
 			return preservedFile(err, file, out)
 		}
 
-		err = registry.WriteConfigDeprecated(newCluster, configBase.Join(registry.PathClusterCompleted), fullCluster)
-		if err != nil {
-			return preservedFile(fmt.Errorf("error writing completed cluster spec: %v", err), file, out)
-		}
-
 		return nil
 	}
 }

diff --git a/cmd/kops/get_cluster.go b/cmd/kops/get_cluster.go
@@ -31,6 +31,7 @@ import (
 	"k8s.io/kops/cmd/kops/util"
 	kopsapi "k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/apis/kops/registry"
+	"k8s.io/kops/pkg/kopscodecs"
 	"k8s.io/kops/util/pkg/tables"
 	"k8s.io/kubectl/pkg/util/i18n"
 	"k8s.io/kubectl/pkg/util/templates"
@@ -283,12 +284,21 @@ func fullClusterSpecs(clusters []*kopsapi.Cluster) ([]*kopsapi.Cluster, error) {
 		if err != nil {
 			return nil, fmt.Errorf("error reading full cluster spec for %q: %v", cluster.ObjectMeta.Name, err)
 		}
-		fullSpec := &kopsapi.Cluster{}
-		err = registry.ReadConfigDeprecated(configBase.Join(registry.PathClusterCompleted), fullSpec)
+		configPath := configBase.Join(registry.PathClusterCompleted)
+		b, err := configPath.ReadFile()
 		if err != nil {
-			return nil, fmt.Errorf("error reading full cluster spec for %q: %v", cluster.ObjectMeta.Name, err)
+			return nil, fmt.Errorf("error loading Cluster %q: %v", configPath, err)
+		}
+
+		o, _, err := kopscodecs.Decode(b, nil)
+		if err != nil {
+			return nil, fmt.Errorf("error parsing Cluster %q: %v", configPath, err)
+		}
+		if fullSpec, ok := o.(*kopsapi.Cluster); ok {
+			fullSpecs = append(fullSpecs, fullSpec)
+		} else {
+			return nil, fmt.Errorf("unexpected object type for Cluster %q: %T", configPath, o)
 		}
-		fullSpecs = append(fullSpecs, fullSpec)
 	}
 	return fullSpecs, nil
 }
diff --git a/pkg/apis/kops/registry/BUILD.bazel b/pkg/apis/kops/registry/BUILD.bazel
diff --git a/pkg/apis/kops/registry/registry.go b/pkg/apis/kops/registry/registry.go
@@ -27,8 +27,8 @@ import (
 const (
 	// Path for the user-specified cluster spec
 	PathCluster = "config"
-	// Path for completed cluster spec in the state store
-	PathClusterCompleted = "cluster.spec"
+	// PathClusterCompleted is the path for completed cluster spec in the state store.
+	PathClusterCompleted = "cluster-completed.spec"
 	// PathKopsVersionUpdated is the path for the version of kops last used to apply the cluster.
 	PathKopsVersionUpdated = "kops-version.txt"
 )

diff --git a/pkg/apis/kops/registry/statestore.go b/pkg/apis/kops/registry/statestore.go
diff --git a/pkg/client/simple/api/clientset.go b/pkg/client/simple/api/clientset.go
@@ -76,6 +76,12 @@ func (c *RESTClientset) UpdateCluster(ctx context.Context, cluster *kops.Cluster
 	return c.KopsClient.Clusters(namespace).Update(ctx, cluster, metav1.UpdateOptions{})
 }
 
+// UpdateCompletedCluster implements the UpdateCompletedCluster method of Clientset for a kubernetes-API state store
+func (c *RESTClientset) UpdateCompletedCluster(ctx context.Context, cluster *kops.Cluster) error {
+	// Not implemented
+	return nil
+}
+
 // ConfigBaseFor implements the ConfigBaseFor method of Clientset for a kubernetes-API state store
 func (c *RESTClientset) ConfigBaseFor(cluster *kops.Cluster) (vfs.Path, error) {
 	if cluster.Spec.ConfigBase != "" {

diff --git a/pkg/client/simple/clientset.go b/pkg/client/simple/clientset.go
@@ -37,6 +37,9 @@ type Clientset interface {
 	// UpdateCluster updates a cluster
 	UpdateCluster(ctx context.Context, cluster *kops.Cluster, status *kops.ClusterStatus) (*kops.Cluster, error)
 
+	// UpdateCompletedCluster updates a completed cluster.
+	UpdateCompletedCluster(ctx context.Context, cluster *kops.Cluster) error
+
 	// ListClusters returns all clusters
 	ListClusters(ctx context.Context, options metav1.ListOptions) (*kops.ClusterList, error)
 

diff --git a/pkg/client/simple/vfsclientset/clientset.go b/pkg/client/simple/vfsclientset/clientset.go
@@ -53,6 +53,11 @@ func (c *VFSClientset) UpdateCluster(ctx context.Context, cluster *kops.Cluster,
 	return c.clusters().Update(cluster, status)
 }
 
+// UpdateCompletedCluster implements the UpdateCluster method of simple.Clientset for a VFS-backed state store.
+func (c *VFSClientset) UpdateCompletedCluster(ctx context.Context, cluster *kops.Cluster) error {
+	return c.clusters().UpdateCompleted(cluster)
+}
+
 // CreateCluster implements the CreateCluster method of simple.Clientset for a VFS-backed state store
 func (c *VFSClientset) CreateCluster(ctx context.Context, cluster *kops.Cluster) (*kops.Cluster, error) {
 	return c.clusters().Create(cluster)
@@ -143,7 +148,8 @@ func DeleteAllClusterState(basePath vfs.Path) error {
 			continue
 		}
 
-		if relativePath == "config" || relativePath == "cluster.spec" || relativePath == registry.PathKopsVersionUpdated {
+		// "cluster.spec" was written by kOps 1.21 and earlier.
+		if relativePath == "config" || relativePath == "cluster.spec" || relativePath == "cluster-completed.spec" || relativePath == registry.PathKopsVersionUpdated {
 			continue
 		}
 		if strings.HasPrefix(relativePath, "addons/") {

diff --git a/pkg/client/simple/vfsclientset/cluster.go b/pkg/client/simple/vfsclientset/cluster.go
@@ -152,6 +152,22 @@ func (r *ClusterVFS) Update(c *api.Cluster, status *api.ClusterStatus) (*api.Clu
 	return c, nil
 }
 
+func (r *ClusterVFS) UpdateCompleted(c *api.Cluster) error {
+	clusterName := c.ObjectMeta.Name
+	if clusterName == "" {
+		return field.Required(field.NewPath("objectMeta", "name"), "clusterName is required")
+	}
+
+	if err := r.writeConfig(c, r.basePath.Join(clusterName, registry.PathClusterCompleted), c); err != nil {
+		if os.IsNotExist(err) {
+			return err
+		}
+		return fmt.Errorf("error writing Cluster: %v", err)
+	}
+
+	return nil
+}
+
 // List returns a slice containing all the cluster names
 // It skips directories that don't look like clusters
 func (r *ClusterVFS) listNames() ([]string, error) {

diff --git a/pkg/model/alimodel/policy_builder.go b/pkg/model/alimodel/policy_builder.go
@@ -276,7 +276,7 @@ func (b *PolicyBuilder) AddOSSPermissions(p *Policy) (*Policy, error) {
 			} else if b.Role == kops.InstanceGroupRoleNode {
 				resources := []string{
 					strings.Join([]string{b.RAMPrefix(), ":oss:*:*:", ramOSSPath, "/addons/*"}, ""),
-					strings.Join([]string{b.RAMPrefix(), ":oss:*:*:", ramOSSPath, "/cluster.spec"}, ""),
+					strings.Join([]string{b.RAMPrefix(), ":oss:*:*:", ramOSSPath, "/cluster-completed.spec"}, ""),
 					strings.Join([]string{b.RAMPrefix(), ":oss:*:*:", ramOSSPath, "/config"}, ""),
 					strings.Join([]string{b.RAMPrefix(), ":oss:*:*:", ramOSSPath, "/instancegroup/*"}, ""),
 					strings.Join([]string{b.RAMPrefix(), ":oss:*:*:", ramOSSPath, "/pki/issued/*"}, ""),

diff --git a/pkg/model/iam/iam_builder.go b/pkg/model/iam/iam_builder.go
@@ -586,7 +586,7 @@ func ReadableStatePaths(cluster *kops.Cluster, role Subject) ([]string, error) {
 	case *NodeRoleNode:
 		paths = append(paths,
 			"/addons/*",
-			"/cluster.spec",
+			"/cluster-completed.spec",
 			"/config",
 			"/igconfig/node/*",
 			"/instancegroup/*",

diff --git a/pkg/model/iam/tests/iam_builder_node_strict.json b/pkg/model/iam/tests/iam_builder_node_strict.json
@@ -24,7 +24,7 @@
       "Effect": "Allow",
       "Resource": [
         "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/addons/*",
-        "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/cluster.spec",
+        "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/cluster-completed.spec",
         "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/config",
         "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/igconfig/node/*",
         "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/instancegroup/*",

diff --git a/pkg/model/iam/tests/iam_builder_node_strict_ecr.json b/pkg/model/iam/tests/iam_builder_node_strict_ecr.json
@@ -24,7 +24,7 @@
       "Effect": "Allow",
       "Resource": [
         "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/addons/*",
-        "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/cluster.spec",
+        "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/cluster-completed.spec",
         "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/config",
         "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/igconfig/node/*",
         "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/instancegroup/*",

diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go
@@ -764,7 +764,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {
 			return fmt.Errorf("error writing kops version: %v", err)
 		}
 
-		err = registry.WriteConfigDeprecated(cluster, configBase.Join(registry.PathClusterCompleted), c.Cluster)
+		err = c.Clientset.UpdateCompletedCluster(ctx, c.Cluster)
 		if err != nil {
 			return fmt.Errorf("error writing completed cluster spec: %v", err)
 		}

diff --git a/upup/pkg/fi/nodeup/BUILD.bazel b/upup/pkg/fi/nodeup/BUILD.bazel