kops-controller: load objects with version conversion

If we deserialize the yaml, we don't go through the version-conversion
logic.  That logic maps from Master -> ControlPlane, so without that
logic we see unexpected values in the "string enums".

cmd/kops-controller/controllers/legacy_node_controller.go

@@ -26,12 +26,11 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/klog/v2"
-	"k8s.io/kops/pkg/apis/kops"
+	api "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"
 	"k8s.io/kops/util/pkg/vfs"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -152,9 +151,9 @@ func (r *LegacyNodeReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-// getClusterForNode returns the kops.Cluster object for the node
+// getClusterForNode returns the api.Cluster object for the node
 // The cluster is actually loaded when we first start
-func (r *LegacyNodeReconciler) getClusterForNode(node *corev1.Node) (*kops.Cluster, error) {
+func (r *LegacyNodeReconciler) getClusterForNode(node *corev1.Node) (*api.Cluster, error) {
 	clusterPath := r.configBase.Join(registry.PathClusterCompleted)
 	cluster, err := r.loadCluster(clusterPath)
 	if err != nil {
@@ -173,8 +172,8 @@ func (r *LegacyNodeReconciler) getInstanceLifecycle(ctx context.Context, node *c
 	return identity.InstanceLifecycle, nil
 }
 
-// getInstanceGroupForNode returns the kops.InstanceGroup object for the node
+// getInstanceGroupForNode returns the api.InstanceGroup object for the node
-func (r *LegacyNodeReconciler) getInstanceGroupForNode(ctx context.Context, node *corev1.Node) (*kops.InstanceGroup, error) {
+func (r *LegacyNodeReconciler) getInstanceGroupForNode(ctx context.Context, node *corev1.Node) (*api.InstanceGroup, error) {
 	// We assume that if the instancegroup label is set, that it is correct
 	// TODO: Should we be paranoid?
 	instanceGroupName := node.Labels["kops.k8s.io/instancegroup"]
@@ -199,8 +198,8 @@ func (r *LegacyNodeReconciler) getInstanceGroupForNode(ctx context.Context, node
 	return r.loadNamedInstanceGroup(instanceGroupName)
 }
 
-// loadCluster loads a kops.Cluster object from a vfs.Path
+// loadCluster loads a api.Cluster object from a vfs.Path
-func (r *LegacyNodeReconciler) loadCluster(p vfs.Path) (*kops.Cluster, error) {
+func (r *LegacyNodeReconciler) loadCluster(p vfs.Path) (*api.Cluster, error) {
 	ttl := time.Hour
 
 	b, err := r.cache.Read(p, ttl)
@@ -212,14 +211,14 @@ func (r *LegacyNodeReconciler) loadCluster(p vfs.Path) (*kops.Cluster, error) {
 	if err != nil {
 		return nil, fmt.Errorf("error parsing Cluster %q: %v", p, err)
 	}
-	if cluster, ok := o.(*kops.Cluster); ok {
+	if cluster, ok := o.(*api.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
+// loadNamedInstanceGroup loads a api.InstanceGroup object from the vfs backing store
-func (r *LegacyNodeReconciler) loadNamedInstanceGroup(name string) (*kops.InstanceGroup, error) {
+func (r *LegacyNodeReconciler) loadNamedInstanceGroup(name string) (*api.InstanceGroup, error) {
 	p := r.configBase.Join("instancegroup", name)
 
 	ttl := time.Hour
@@ -228,9 +227,14 @@ func (r *LegacyNodeReconciler) loadNamedInstanceGroup(name string) (*kops.Instan
 		return nil, fmt.Errorf("error loading InstanceGroup %q: %v", p, err)
 	}
 
-	instanceGroup := &kops.InstanceGroup{}
+	object, _, err := kopscodecs.Decode(b, nil)
-	if err := utils.YamlUnmarshal(b, instanceGroup); err != nil {
+	if err != nil {
-		return nil, fmt.Errorf("error parsing InstanceGroup %q: %v", p, err)
+		return nil, fmt.Errorf("error parsing %s: %w", p, err)
+	}
+
+	instanceGroup, ok := object.(*api.InstanceGroup)
+	if !ok {
+		return nil, fmt.Errorf("unexpected type, expected InstanceGroup, got %T", object)
 	}
 
 	return instanceGroup, nil

pkg/nodelabels/builder.go

@@ -19,7 +19,7 @@ package nodelabels
 import (
 	"fmt"
 
-	"k8s.io/kops/pkg/apis/kops"
+	api "k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/featureflag"
 	"k8s.io/kops/util/pkg/reflectutils"
 )
@@ -39,25 +39,25 @@ const (
 
 // BuildNodeLabels returns the node labels for the specified instance group
 // This moved from the kubelet to a central controller in kubernetes 1.16
-func BuildNodeLabels(cluster *kops.Cluster, instanceGroup *kops.InstanceGroup) (map[string]string, error) {
+func BuildNodeLabels(cluster *api.Cluster, instanceGroup *api.InstanceGroup) (map[string]string, error) {
 	isControlPlane := false
 	isAPIServer := false
 	isNode := false
 	switch instanceGroup.Spec.Role {
-	case kops.InstanceGroupRoleControlPlane:
+	case api.InstanceGroupRoleControlPlane:
 		isControlPlane = true
-	case kops.InstanceGroupRoleAPIServer:
+	case api.InstanceGroupRoleAPIServer:
 		isAPIServer = true
-	case kops.InstanceGroupRoleNode:
+	case api.InstanceGroupRoleNode:
 		isNode = true
-	case kops.InstanceGroupRoleBastion:
+	case api.InstanceGroupRoleBastion:
 		// no labels to add
 	default:
 		return nil, fmt.Errorf("unhandled instanceGroup role %q", instanceGroup.Spec.Role)
 	}
 
 	// Merge KubeletConfig for NodeLabels
-	c := &kops.KubeletConfigSpec{}
+	c := &api.KubeletConfigSpec{}
 	if isControlPlane {
 		reflectutils.JSONMergeStruct(c, cluster.Spec.ControlPlaneKubelet)
 	} else {
@@ -116,7 +116,7 @@ func BuildNodeLabels(cluster *kops.Cluster, instanceGroup *kops.InstanceGroup) (
 		nodeLabels[k] = v
 	}
 
-	if instanceGroup.Spec.Manager == kops.InstanceManagerKarpenter {
+	if instanceGroup.Spec.Manager == api.InstanceManagerKarpenter {
 		nodeLabels["karpenter.sh/provisioner-name"] = instanceGroup.ObjectMeta.Name
 	}
 

upup/pkg/fi/utils/yaml.go

@@ -28,6 +28,9 @@ func YAMLToJSON(yamlBytes []byte) ([]byte, error) {
 }
 
 // YamlUnmarshal unmarshals the yaml content to an interface
+// Note: if you are loading a kops.k8s.io API object,
+// normally you want something like kopscodecs.Decode,
+// so that we can convert between apiVersions.
 func YamlUnmarshal(yamlBytes []byte, dest interface{}) error {
 	return yaml.Unmarshal(yamlBytes, dest)
 }