Merge pull request #17703 from justinsb/clusterapi_controllers_refactor_nodeup

capi: refactor to use shared nodeup builder

Author: k8s-ci-robot

cmd/kops-controller/main.go

@@ -37,6 +37,7 @@ import (
 	"k8s.io/kops/pkg/apis/kops/v1alpha2"
 	"k8s.io/kops/pkg/bootstrap"
 	"k8s.io/kops/pkg/bootstrap/pkibootstrap"
+	"k8s.io/kops/pkg/client/simple"
 	"k8s.io/kops/pkg/controllers/clusterapi"
 	"k8s.io/kops/pkg/nodeidentity"
 	nodeidentityaws "k8s.io/kops/pkg/nodeidentity/aws"
@@ -136,6 +137,8 @@ func main() {
 		capiManager = nodeidentityclusterapi.NewManager(mgr.GetClient())
 	}
 
+	var clientset simple.Clientset
+
 	if opt.Server != nil {
 		var verifiers []bootstrap.Verifier
 		var err error
@@ -225,6 +228,7 @@ func main() {
 			setupLog.Error(err, "unable to create server")
 			os.Exit(1)
 		}
+		clientset = srv.GetClientset()
 		mgr.Add(srv)
 	}
 
@@ -249,7 +253,11 @@ func main() {
 	// +kubebuilder:scaffold:builder
 
 	if opt.CAPI.IsEnabled() {
-		if err := clusterapi.RegisterControllers(mgr); err != nil {
+		if clientset == nil {
+			setupLog.Error(fmt.Errorf("clientset is not initialized"), "registering Cluster API controllers")
+			os.Exit(1)
+		}
+		if err := clusterapi.RegisterControllers(mgr, clientset); err != nil {
 			setupLog.Error(err, "registering Cluster API controllers")
 			os.Exit(1)
 		}

cmd/kops-controller/pkg/server/server.go

@@ -126,6 +126,10 @@ func NewServer(vfsContext *vfs.VFSContext, opt *config.Options, verifier bootstr
 	return s, nil
 }
 
+func (s *Server) GetClientset() simple.Clientset {
+	return s.clientset
+}
+
 func (s *Server) NeedLeaderElection() bool {
 	return false
 }

pkg/commands/toolbox_enroll.go

@@ -423,11 +423,11 @@ type BootstrapData struct {
 // ConfigBuilder builds bootstrap configuration for a node.
 type ConfigBuilder struct {
 	// ClusterName is the name of the cluster to build.
-	// Required.
+	// Required (unless Cluster is set).
 	ClusterName string
 
 	// InstanceGroupName is the name of the InstanceGroup we are building configuration for.
-	// Required.
+	// Required (unless InstanceGroup is set).
 	InstanceGroupName string
 
 	// Clientset is the clientset to use to query for clusters / instancegroups etc

pkg/controllers/clusterapi/kopsconfig_controller.go

@@ -17,10 +17,8 @@ limitations under the License.
 package clusterapi
 
 import (
-	"bytes"
 	"context"
 	"fmt"
-	"sort"
 
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -31,29 +29,24 @@ import (
 	capikops "k8s.io/kops/clusterapi/controlplane/kops/api/v1beta1"
 	clusterv1 "k8s.io/kops/clusterapi/snapshot/cluster-api/api/v1beta1"
 	"k8s.io/kops/pkg/apis/kops"
-	"k8s.io/kops/pkg/apis/kops/registry"
 	kopsapi "k8s.io/kops/pkg/apis/kops/v1alpha2"
-	"k8s.io/kops/pkg/apis/nodeup"
-	"k8s.io/kops/pkg/assets"
-	"k8s.io/kops/pkg/client/simple/vfsclientset"
+	"k8s.io/kops/pkg/client/simple"
+	"k8s.io/kops/pkg/commands"
 	"k8s.io/kops/pkg/kopscodecs"
 	"k8s.io/kops/pkg/model"
-	"k8s.io/kops/pkg/model/resources"
-	"k8s.io/kops/pkg/nodemodel"
 	"k8s.io/kops/pkg/wellknownservices"
-	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup"
-	"k8s.io/kops/util/pkg/vfs"
 	"k8s.io/utils/pointer"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
 // NewKopsConfigReconciler is the constructor for a KopsConfigReconciler
-func NewKopsConfigReconciler(mgr manager.Manager) error {
+func NewKopsConfigReconciler(mgr manager.Manager, clientset simple.Clientset) error {
 	r := &KopsConfigReconciler{
-		client: mgr.GetClient(),
+		client:    mgr.GetClient(),
+		clientset: clientset,
 	}
 
 	return ctrl.NewControllerManagedBy(mgr).
@@ -65,6 +58,9 @@ func NewKopsConfigReconciler(mgr manager.Manager) error {
 type KopsConfigReconciler struct {
 	// client is the controller-runtime client
 	client client.Client
+
+	// clientset is a kops clientset
+	clientset simple.Clientset
 }
 
 // +kubebuilder:rbac:groups=,resources=nodes,verbs=get;list;watch;patch
@@ -176,25 +172,6 @@ func (r *KopsConfigReconciler) storeBootstrapData(ctx context.Context, parent *a
 }
 
 func (r *KopsConfigReconciler) buildBootstrapData(ctx context.Context, cluster *kopsapi.Cluster, kopsControlPlane *capikops.KopsControlPlane) ([]byte, error) {
-
-	config, err := BuildNodeupConfig(ctx, cluster, kopsControlPlane)
-	if err != nil {
-		return nil, err
-	}
-	return config.NodeupScript, nil
-}
-
-type NodeupConfig struct {
-	NodeupScript []byte
-	NodeupConfig *nodeup.Config
-}
-
-// TODO: Dedup with b.builder.NodeUpConfigBuilder.BuildConfig
-func BuildNodeupConfig(ctx context.Context, cluster *kopsapi.Cluster, kopsControlPlane *capikops.KopsControlPlane) (*NodeupConfig, error) {
-	// tf := &TemplateFunctions{
-	// 	KopsModelContext: *modelContext,
-	// 	cloud:            cloud,
-	// }
 	wellKnownAddresses := model.WellKnownAddresses{}
 	for _, systemEndpoint := range kopsControlPlane.Status.SystemEndpoints {
 		switch systemEndpoint.Type {
@@ -205,210 +182,43 @@ func BuildNodeupConfig(ctx context.Context, cluster *kopsapi.Cluster, kopsContro
 		}
 	}
 
-	// TODO: Sync with other nodeup config builder
 	clusterInternal := &kops.Cluster{}
-	if err := kopscodecs.Scheme.Convert(cluster, clusterInternal, nil); err != nil {
-		return nil, fmt.Errorf("converting cluster object: %w", err)
-	}
-	// TODO: Fix validation
-	clusterInternal.Namespace = ""
-
-	// if clusterInternal.Spec.KubeAPIServer == nil {
-	// 	clusterInternal.Spec.KubeAPIServer = &kops.KubeAPIServerConfig{}
-	// }
-
-	// cluster := &kops.Cluster{}
-	// cluster.Spec.KubernetesVersion = "1.28.3"
-	// cluster.Spec.KubeAPIServer = &kops.KubeAPIServerConfig{}
-
-	// if cluster.Spec.KubeAPIServer == nil {
-	// 	cluster.Spec.KubeAPIServer = &kopsapi.KubeAPIServerConfig{}
-	// }
-
-	vfsContext := vfs.NewVFSContext()
-
-	basePath, err := registry.ConfigBase(vfsContext, clusterInternal)
-	if err != nil {
-		return nil, fmt.Errorf("parsing vfs base path: %w", err)
-	}
 
-	clientset := vfsclientset.NewVFSClientset(vfsContext, basePath)
+	configBuilder := &commands.ConfigBuilder{}
+	configBuilder.Clientset = r.clientset
 
-	ig := &kops.InstanceGroup{}
-	// TODO: Name
-	ig.SetName("todo-ig-name")
-	ig.Spec.Role = kops.InstanceGroupRoleNode
-
-	getAssets := false
-	assetBuilder := assets.NewAssetBuilder(vfsContext, clusterInternal.Spec.Assets, getAssets)
-
-	cloud, err := cloudup.BuildCloud(clusterInternal)
-	if err != nil {
-		return nil, fmt.Errorf("building cloud: %w", err)
-	}
-
-	// assetBuilder := assets.NewAssetBuilder(clientset.VFSContext(), cluster.Spec.Assets, cluster.Spec.KubernetesVersion, false)
-	var instanceGroups []*kops.InstanceGroup
-	instanceGroups = append(instanceGroups, ig)
-
-	fullCluster, err := cloudup.PopulateClusterSpec(ctx, clientset, clusterInternal, instanceGroups, cloud, assetBuilder)
-	if err != nil {
-		return nil, fmt.Errorf("building full cluster spec: %w", err)
-	}
-
-	channel, err := cloudup.ChannelForCluster(clientset.VFSContext(), fullCluster)
-	if err != nil {
-		// TODO: Maybe this should be a warning
-		return nil, fmt.Errorf("building channel for cluster: %w", err)
-	}
-
-	var fullInstanceGroups []*kops.InstanceGroup
-	for _, instanceGroup := range instanceGroups {
-		fullGroup, err := cloudup.PopulateInstanceGroupSpec(fullCluster, instanceGroup, cloud, channel)
-		if err != nil {
-			return nil, fmt.Errorf("building full instance group spec: %w", err)
+	{
+		if err := kopscodecs.Scheme.Convert(cluster, clusterInternal, nil); err != nil {
+			return nil, fmt.Errorf("converting cluster object: %w", err)
 		}
-		fullInstanceGroups = append(fullInstanceGroups, fullGroup)
-	}
-
-	encryptionConfigSecretHash := ""
-	// if fi.ValueOf(c.Cluster.Spec.EncryptionConfig) {
-	// 	secret, err := secretStore.FindSecret("encryptionconfig")
-	// 	if err != nil {
-	// 		return fmt.Errorf("could not load encryptionconfig secret: %v", err)
-	// 	}
-	// 	if secret == nil {
-	// 		fmt.Println("")
-	// 		fmt.Println("You have encryptionConfig enabled, but no encryptionconfig secret has been set.")
-	// 		fmt.Println("See `kops create secret encryptionconfig -h` and https://kubernetes.io/docs/tasks/administer-cluster/encrypt-data/")
-	// 		return fmt.Errorf("could not find encryptionconfig secret")
-	// 	}
-	// 	hashBytes := sha256.Sum256(secret.Data)
-	// 	encryptionConfigSecretHash = base64.URLEncoding.EncodeToString(hashBytes[:])
-	// }
-
-	nodeUpAssets, err := nodemodel.BuildNodeUpAssets(ctx, assetBuilder)
-	if err != nil {
-		return nil, err
-	}
+		// TODO: Fix validation
+		clusterInternal.Namespace = ""
 
-	configBuilder, err := nodemodel.NewNodeUpConfigBuilder(fullCluster, assetBuilder, encryptionConfigSecretHash)
-	if err != nil {
-		return nil, fmt.Errorf("building node config: %w", err)
-	}
-
-	// bootstrapScript := &model.BootstrapScript{
-	// 	// KopsModelContext:    modelContext,
-	// 	Lifecycle: fi.LifecycleSync,
-	// 	// NodeUpConfigBuilder: configBuilder,
-	// 	// NodeUpAssets:        c.NodeUpAssets,
-	// }
-
-	keysets := make(map[string]*fi.Keyset)
-
-	// var keystoreBase vfs.Path
-
-	// if cluster.Spec.ConfigStore.Keypairs == "" {
-	// 	configBase, err := registry.ConfigBase(vfsContext, clusterInternal)
-	// 	if err != nil {
-	// 		return nil, err
-	// 	}
-	// 	keystoreBase = configBase.Join("pki")
-	// } else {
-	// 	storePath, err := vfsContext.BuildVfsPath(cluster.Spec.ConfigStore.Keypairs)
-	// 	if err != nil {
-	// 		return nil, err
-	// 	}
-	// 	keystoreBase = storePath
-	// }
-
-	// keystore := fi.NewVFSCAStore(clusterInternal, keystoreBase)
-	keystore, err := clientset.KeyStore(fullCluster)
-	if err != nil {
-		return nil, err
+		configBuilder.Cluster = clusterInternal
+		configBuilder.ClusterName = clusterInternal.Name
 	}
 
-	for _, keyName := range []string{"kubernetes-ca"} {
-		keyset, err := keystore.FindKeyset(ctx, keyName)
-		if err != nil {
-			return nil, fmt.Errorf("getting keyset %q: %w", keyName, err)
-		}
-
-		if keyset == nil {
-			return nil, fmt.Errorf("failed to find keyset %q", keyName)
-		}
+	ig := &kops.InstanceGroup{}
+	{
+		ig.SetName("placeholder-ig-name") // IG name is not used for nodeup config generation
+		ig.Spec.Role = kops.InstanceGroupRoleNode
 
-		keysets[keyName] = keyset
+		configBuilder.InstanceGroup = ig
+		configBuilder.InstanceGroupName = ig.Name
 	}
 
-	nodeupConfig, bootConfig, err := configBuilder.BuildConfig(fullInstanceGroups[0], wellKnownAddresses, keysets)
-	if err != nil {
-		return nil, err
-	}
-
-	// configData, err := utils.YamlMarshal(config)
-	// if err != nil {
-	// 	return nil, fmt.Errorf("error converting nodeup config to yaml: %v", err)
-	// }
-	// sum256 := sha256.Sum256(configData)
-	// bootConfig.NodeupConfigHash = base64.StdEncoding.EncodeToString(sum256[:])
-	// b.nodeupConfig.Resource = fi.NewBytesResource(configData)
-
-	var nodeupScript resources.NodeUpScript
-	nodeupScript.NodeUpAssets = nodeUpAssets.NodeUpAssets
-	// nodeupScript.NodeUpAssets = configBuilder.NodeUpAssets()
-	nodeupScript.BootConfig = bootConfig
-
 	{
-		nodeupScript.EnvironmentVariables = func() (string, error) {
-			env := make(map[string]string)
-
-			// env, err := b.buildEnvironmentVariables()
-			// if err != nil {
-			// 	return "", err
-			// }
-
-			// Sort keys to have a stable sequence of "export xx=xxx"" statements
-			var keys []string
-			for k := range env {
-				keys = append(keys, k)
-			}
-			sort.Strings(keys)
-
-			var b bytes.Buffer
-			for _, k := range keys {
-				b.WriteString(fmt.Sprintf("export %s=%s\n", k, env[k]))
-			}
-			return b.String(), nil
-		}
-
-		nodeupScript.ProxyEnv = func() (string, error) {
-			return "", nil
-			// return b.createProxyEnv(cluster.Spec.Networking.EgressProxy)
+		cloud, err := cloudup.BuildCloud(clusterInternal)
+		if err != nil {
+			return nil, fmt.Errorf("building cloud: %w", err)
 		}
+		configBuilder.Cloud = cloud
 	}
 
-	// TODO: nodeupScript.CompressUserData = fi.ValueOf(b.ig.Spec.CompressUserData)
-
-	// By setting some sysctls early, we avoid broken configurations that prevent nodeup download.
-	// See https://github.com/kubernetes/kops/issues/10206 for details.
-	// TODO: nodeupScript.SetSysctls = setSysctls()
-
-	// nodeupScript.CloudProvider = string(cluster.Spec.GetCloudProvider())
-	nodeupScript.CloudProvider = string(clusterInternal.GetCloudProvider())
-
-	nodeupScriptResource, err := nodeupScript.Build()
-	if err != nil {
-		return nil, err
-	}
-
-	b, err := fi.ResourceAsBytes(nodeupScriptResource)
+	bootstrapData, err := configBuilder.GetBootstrapData(ctx)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("building bootstrap data: %w", err)
 	}
 
-	return &NodeupConfig{
-		NodeupScript: b,
-		NodeupConfig: nodeupConfig,
-	}, nil
+	return bootstrapData.NodeupScript, nil
 }

pkg/controllers/clusterapi/register.go

@@ -19,11 +19,12 @@ package clusterapi
 import (
 	"fmt"
 
+	"k8s.io/kops/pkg/client/simple"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
-func RegisterControllers(mgr manager.Manager) error {
-	if err := NewKopsConfigReconciler(mgr); err != nil {
+func RegisterControllers(mgr manager.Manager, clientset simple.Clientset) error {
+	if err := NewKopsConfigReconciler(mgr, clientset); err != nil {
 		return fmt.Errorf("error creating KopsConfig controller: %w", err)
 	}