Merge pull request #16954 from justinsb/refactor_file_assets

refactor: build assets for kubernetes and nodeup explicitly

clusterapi/bootstrap/controllers/kopsconfig_controller.go

@@ -35,10 +35,8 @@ import (
 	"k8s.io/kops/pkg/model"
 	"k8s.io/kops/pkg/model/resources"
 	"k8s.io/kops/pkg/nodemodel"
-	"k8s.io/kops/pkg/nodemodel/wellknownassets"
 	"k8s.io/kops/pkg/wellknownservices"
 	"k8s.io/kops/upup/pkg/fi"
-	"k8s.io/kops/util/pkg/architectures"
 	"k8s.io/kops/util/pkg/vfs"
 	"k8s.io/utils/pointer"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -211,17 +209,12 @@ func (r *KopsConfigReconciler) buildBootstrapData(ctx context.Context) ([]byte,
 	// 	encryptionConfigSecretHash = base64.URLEncoding.EncodeToString(hashBytes[:])
 	// }
 
-	nodeUpAssets := make(map[architectures.Architecture]*assets.MirroredAsset)
+	nodeUpAssets, err := nodemodel.BuildNodeUpAssets(ctx, assetBuilder)
-	for _, arch := range architectures.GetSupported() {
-		asset, err := wellknownassets.NodeUpAsset(assetBuilder, arch)
 	if err != nil {
 		return nil, err
 	}
-		nodeUpAssets[arch] = asset
-	}
 
-	assets := make(map[architectures.Architecture][]*assets.MirroredAsset)
+	configBuilder, err := nodemodel.NewNodeUpConfigBuilder(cluster, assetBuilder, encryptionConfigSecretHash)
-	configBuilder, err := nodemodel.NewNodeUpConfigBuilder(cluster, assetBuilder, assets, encryptionConfigSecretHash)
 	if err != nil {
 		return nil, err
 	}
@@ -267,7 +260,7 @@ func (r *KopsConfigReconciler) buildBootstrapData(ctx context.Context) ([]byte,
 	// b.nodeupConfig.Resource = fi.NewBytesResource(configData)
 
 	var nodeupScript resources.NodeUpScript
-	nodeupScript.NodeUpAssets = nodeUpAssets
+	nodeupScript.NodeUpAssets = nodeUpAssets.NodeUpAssets
 	nodeupScript.BootConfig = bootConfig
 
 	{

pkg/commands/toolbox_enroll.go

@@ -752,14 +752,13 @@ func (b *ConfigBuilder) GetBootstrapData(ctx context.Context) (*BootstrapData, e
 	// 	}
 	// 	hashBytes := sha256.Sum256(secret.Data)
 	// 	encryptionConfigSecretHash = base64.URLEncoding.EncodeToString(hashBytes[:])
-	// }
 
-	fileAssets := &nodemodel.FileAssets{Cluster: cluster}
+	nodeUpAssets, err := nodemodel.BuildNodeUpAssets(ctx, assetBuilder)
-	if err := fileAssets.AddFileAssets(assetBuilder); err != nil {
+	if err != nil {
 		return nil, err
 	}
 
-	configBuilder, err := nodemodel.NewNodeUpConfigBuilder(cluster, assetBuilder, fileAssets.Assets, encryptionConfigSecretHash)
+	configBuilder, err := nodemodel.NewNodeUpConfigBuilder(cluster, assetBuilder, encryptionConfigSecretHash)
 	if err != nil {
 		return nil, err
 	}
@@ -791,7 +790,7 @@ func (b *ConfigBuilder) GetBootstrapData(ctx context.Context) (*BootstrapData, e
 	}
 
 	var nodeupScript resources.NodeUpScript
-	nodeupScript.NodeUpAssets = fileAssets.NodeUpAssets
+	nodeupScript.NodeUpAssets = nodeUpAssets.NodeUpAssets
 	nodeupScript.BootConfig = bootConfig
 
 	nodeupScript.WithEnvironmentVariables(cluster, ig)

pkg/nodemodel/fileassets.go

@@ -17,6 +17,7 @@ limitations under the License.
 package nodemodel
 
 import (
+	"context"
 	"fmt"
 	"net/url"
 	"path"
@@ -32,70 +33,62 @@ import (
 	"k8s.io/kops/util/pkg/hashing"
 )
 
-type FileAssets struct {
+// KubernetesFileAssets are the assets for downloading Kubernetes binaries
-	// Assets is a list of sources for files (primarily when not using everything containerized)
+type KubernetesFileAssets struct {
-	// Formats:
+	// KubernetesFileAssets are the assets for downloading Kubernetes binaries
-	//  raw url: http://... or https://...
+	KubernetesFileAssets map[architectures.Architecture][]*assets.MirroredAsset
-	//  url with hash: <hex>@http://... or <hex>@https://...
-	Assets map[architectures.Architecture][]*assets.MirroredAsset
-
-	// NodeUpAssets are the assets for downloading nodeup
-	NodeUpAssets map[architectures.Architecture]*assets.MirroredAsset
-
-	Cluster *kops.Cluster
 }
 
-// AddFileAssets adds the file assets within the assetBuilder
+// BuildKubernetesFileAssets returns the Kubernetes file assets for the given cluster
-func (c *FileAssets) AddFileAssets(assetBuilder *assets.AssetBuilder) error {
+func BuildKubernetesFileAssets(cluster *kops.Cluster, assetBuilder *assets.AssetBuilder) (*KubernetesFileAssets, error) {
 	var baseURL string
-	if components.IsBaseURL(c.Cluster.Spec.KubernetesVersion) {
+	if components.IsBaseURL(cluster.Spec.KubernetesVersion) {
-		baseURL = c.Cluster.Spec.KubernetesVersion
+		baseURL = cluster.Spec.KubernetesVersion
 	} else {
-		baseURL = "https://dl.k8s.io/release/v" + c.Cluster.Spec.KubernetesVersion
+		baseURL = "https://dl.k8s.io/release/v" + cluster.Spec.KubernetesVersion
 	}
 
-	c.Assets = make(map[architectures.Architecture][]*assets.MirroredAsset)
+	kubernetesAssets := make(map[architectures.Architecture][]*assets.MirroredAsset)
-	c.NodeUpAssets = make(map[architectures.Architecture]*assets.MirroredAsset)
 	for _, arch := range architectures.GetSupported() {
-		c.Assets[arch] = []*assets.MirroredAsset{}
+		kubernetesAssets[arch] = []*assets.MirroredAsset{}
 
 		k8sAssetsNames := []string{
 			fmt.Sprintf("/bin/linux/%s/kubelet", arch),
 			fmt.Sprintf("/bin/linux/%s/kubectl", arch),
 		}
 
-		if needsMounterAsset(c.Cluster) {
+		if needsMounterAsset(cluster) {
 			k8sAssetsNames = append(k8sAssetsNames, fmt.Sprintf("/bin/linux/%s/mounter", arch))
 		}
 
 		for _, an := range k8sAssetsNames {
 			k, err := url.Parse(baseURL)
 			if err != nil {
-				return err
+				return nil, err
 			}
 			k.Path = path.Join(k.Path, an)
 
 			asset, err := assetBuilder.RemapFile(k, nil)
 			if err != nil {
-				return err
+				return nil, err
 			}
-			c.Assets[arch] = append(c.Assets[arch], assets.BuildMirroredAsset(asset))
+			kubernetesAssets[arch] = append(kubernetesAssets[arch], assets.BuildMirroredAsset(asset))
 		}
 
-		kubernetesVersion, _ := util.ParseKubernetesVersion(c.Cluster.Spec.KubernetesVersion)
+		kubernetesVersion, _ := util.ParseKubernetesVersion(cluster.Spec.KubernetesVersion)
 
-		cloudProvider := c.Cluster.GetCloudProvider()
+		cloudProvider := cluster.GetCloudProvider()
 		if ok := model.UseExternalKubeletCredentialProvider(*kubernetesVersion, cloudProvider); ok {
 			switch cloudProvider {
 			case kops.CloudProviderGCE:
-				binaryLocation := c.Cluster.Spec.CloudProvider.GCE.BinariesLocation
+				binaryLocation := cluster.Spec.CloudProvider.GCE.BinariesLocation
 				if binaryLocation == nil {
 					binaryLocation = fi.PtrTo("https://storage.googleapis.com/k8s-staging-cloud-provider-gcp/auth-provider-gcp")
 				}
 				// VALID FOR 60 DAYS WE REALLY NEED TO MERGE https://github.com/kubernetes/cloud-provider-gcp/pull/601 and CUT A RELEASE
 				k, err := url.Parse(fmt.Sprintf("%s/linux-%s/v20231005-providersv0.27.1-65-g8fbe8d27", *binaryLocation, arch))
 				if err != nil {
-					return err
+					return nil, err
 				}
 
 				// TODO: Move these hashes to assetdata
@@ -105,81 +98,98 @@ func (c *FileAssets) AddFileAssets(assetBuilder *assets.AssetBuilder) error {
 				}
 				hash, err := hashing.FromString(hashes[arch])
 				if err != nil {
-					return fmt.Errorf("unable to parse auth-provider-gcp binary asset hash %q: %v", hashes[arch], err)
+					return nil, fmt.Errorf("unable to parse auth-provider-gcp binary asset hash %q: %v", hashes[arch], err)
 				}
 				asset, err := assetBuilder.RemapFile(k, hash)
 				if err != nil {
-					return err
+					return nil, err
 				}
 
-				c.Assets[arch] = append(c.Assets[arch], assets.BuildMirroredAsset(asset))
+				kubernetesAssets[arch] = append(kubernetesAssets[arch], assets.BuildMirroredAsset(asset))
 			case kops.CloudProviderAWS:
-				binaryLocation := c.Cluster.Spec.CloudProvider.AWS.BinariesLocation
+				binaryLocation := cluster.Spec.CloudProvider.AWS.BinariesLocation
 				if binaryLocation == nil {
 					binaryLocation = fi.PtrTo("https://artifacts.k8s.io/binaries/cloud-provider-aws/v1.27.1")
 				}
 
 				u, err := url.Parse(fmt.Sprintf("%s/linux/%s/ecr-credential-provider-linux-%s", *binaryLocation, arch, arch))
 				if err != nil {
-					return err
+					return nil, err
 				}
 				asset, err := assetBuilder.RemapFile(u, nil)
 				if err != nil {
-					return err
+					return nil, err
 				}
-				c.Assets[arch] = append(c.Assets[arch], assets.BuildMirroredAsset(asset))
+				kubernetesAssets[arch] = append(kubernetesAssets[arch], assets.BuildMirroredAsset(asset))
 			}
 		}
 
 		{
-			cniAsset, err := wellknownassets.FindCNIAssets(c.Cluster, assetBuilder, arch)
+			cniAsset, err := wellknownassets.FindCNIAssets(cluster, assetBuilder, arch)
 			if err != nil {
-				return err
+				return nil, err
 			}
-			c.Assets[arch] = append(c.Assets[arch], assets.BuildMirroredAsset(cniAsset))
+			kubernetesAssets[arch] = append(kubernetesAssets[arch], assets.BuildMirroredAsset(cniAsset))
 		}
 
-		if c.Cluster.Spec.Containerd == nil || !c.Cluster.Spec.Containerd.SkipInstall {
+		if cluster.Spec.Containerd == nil || !cluster.Spec.Containerd.SkipInstall {
-			containerdAsset, err := wellknownassets.FindContainerdAsset(c.Cluster, assetBuilder, arch)
+			containerdAsset, err := wellknownassets.FindContainerdAsset(cluster, assetBuilder, arch)
 			if err != nil {
-				return err
+				return nil, err
 			}
 			if containerdAsset != nil {
-				c.Assets[arch] = append(c.Assets[arch], assets.BuildMirroredAsset(containerdAsset))
+				kubernetesAssets[arch] = append(kubernetesAssets[arch], assets.BuildMirroredAsset(containerdAsset))
 			}
 
-			runcAsset, err := wellknownassets.FindRuncAsset(c.Cluster, assetBuilder, arch)
+			runcAsset, err := wellknownassets.FindRuncAsset(cluster, assetBuilder, arch)
 			if err != nil {
-				return err
+				return nil, err
 			}
 			if runcAsset != nil {
-				c.Assets[arch] = append(c.Assets[arch], assets.BuildMirroredAsset(runcAsset))
+				kubernetesAssets[arch] = append(kubernetesAssets[arch], assets.BuildMirroredAsset(runcAsset))
 			}
-			nerdctlAsset, err := wellknownassets.FindNerdctlAsset(c.Cluster, assetBuilder, arch)
+			nerdctlAsset, err := wellknownassets.FindNerdctlAsset(cluster, assetBuilder, arch)
 			if err != nil {
-				return err
+				return nil, err
 			}
 			if nerdctlAsset != nil {
-				c.Assets[arch] = append(c.Assets[arch], assets.BuildMirroredAsset(nerdctlAsset))
+				kubernetesAssets[arch] = append(kubernetesAssets[arch], assets.BuildMirroredAsset(nerdctlAsset))
 			}
 		}
 
-		crictlAsset, err := wellknownassets.FindCrictlAsset(c.Cluster, assetBuilder, arch)
+		crictlAsset, err := wellknownassets.FindCrictlAsset(cluster, assetBuilder, arch)
 		if err != nil {
-			return err
+			return nil, err
 		}
 		if crictlAsset != nil {
-			c.Assets[arch] = append(c.Assets[arch], assets.BuildMirroredAsset(crictlAsset))
+			kubernetesAssets[arch] = append(kubernetesAssets[arch], assets.BuildMirroredAsset(crictlAsset))
 		}
 
+	}
+
+	return &KubernetesFileAssets{
+		KubernetesFileAssets: kubernetesAssets,
+	}, nil
+}
+
+// NodeUpAssets are the assets for downloading nodeup
+type NodeUpAssets struct {
+	// NodeUpAssets are the assets for downloading nodeup
+	NodeUpAssets map[architectures.Architecture]*assets.MirroredAsset
+}
+
+func BuildNodeUpAssets(ctx context.Context, assetBuilder *assets.AssetBuilder) (*NodeUpAssets, error) {
+	nodeUpAssets := make(map[architectures.Architecture]*assets.MirroredAsset)
+	for _, arch := range architectures.GetSupported() {
 		asset, err := wellknownassets.NodeUpAsset(assetBuilder, arch)
 		if err != nil {
-			return err
+			return nil, err
 		}
-		c.NodeUpAssets[arch] = asset
+		nodeUpAssets[arch] = asset
 	}
-
+	return &NodeUpAssets{
-	return nil
+		NodeUpAssets: nodeUpAssets,
+	}, nil
 }
 
 // needsMounterAsset checks if we need the mounter program

pkg/nodemodel/nodeupconfigbuilder.go

@@ -42,12 +42,6 @@ import (
 )
 
 type nodeUpConfigBuilder struct {
-	// Assets is a list of sources for files (primarily when not using everything containerized)
-	// Formats:
-	//  raw url: http://... or https://...
-	//  url with hash: <hex>@http://... or <hex>@https://...
-	assets map[architectures.Architecture][]*assets.MirroredAsset
-
 	assetBuilder               *assets.AssetBuilder
 	channels                   []string
 	configBase                 vfs.Path
@@ -59,7 +53,7 @@ type nodeUpConfigBuilder struct {
 	encryptionConfigSecretHash string
 }
 
-func NewNodeUpConfigBuilder(cluster *kops.Cluster, assetBuilder *assets.AssetBuilder, nodeAssets map[architectures.Architecture][]*assets.MirroredAsset, encryptionConfigSecretHash string) (model.NodeUpConfigBuilder, error) {
+func NewNodeUpConfigBuilder(cluster *kops.Cluster, assetBuilder *assets.AssetBuilder, encryptionConfigSecretHash string) (model.NodeUpConfigBuilder, error) {
 	configBase, err := vfs.Context.BuildVfsPath(cluster.Spec.ConfigStore.Base)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing configStore.base %q: %v", cluster.Spec.ConfigStore.Base, err)
@@ -194,7 +188,6 @@ func NewNodeUpConfigBuilder(cluster *kops.Cluster, assetBuilder *assets.AssetBui
 
 	configBuilder := nodeUpConfigBuilder{
 		assetBuilder:               assetBuilder,
-		assets:                     nodeAssets,
 		channels:                   channels,
 		configBase:                 configBase,
 		cluster:                    cluster,
@@ -227,10 +220,14 @@ func (n *nodeUpConfigBuilder) BuildConfig(ig *kops.InstanceGroup, wellKnownAddre
 
 	config, bootConfig := nodeup.NewConfig(cluster, ig)
 
+	kubernetesAssets, err := BuildKubernetesFileAssets(cluster, n.assetBuilder)
+	if err != nil {
+		return nil, nil, err
+	}
 	config.Assets = make(map[architectures.Architecture][]string)
 	for _, arch := range architectures.GetSupported() {
 		config.Assets[arch] = []string{}
-		for _, a := range n.assets[arch] {
+		for _, a := range kubernetesAssets.KubernetesFileAssets[arch] {
 			config.Assets[arch] = append(config.Assets[arch], a.CompactString())
 		}
 	}

upup/pkg/fi/cloudup/apply_cluster.go

@@ -385,11 +385,6 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) (*ApplyResults, error) {
 		}
 	}
 
-	fileAssets := &nodemodel.FileAssets{Cluster: cluster}
-	if err := fileAssets.AddFileAssets(assetBuilder); err != nil {
-		return nil, err
-	}
-
 	project := ""
 	scwZone := ""
 
@@ -513,7 +508,11 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) (*ApplyResults, error) {
 		cloud:            cloud,
 	}
 
-	configBuilder, err := nodemodel.NewNodeUpConfigBuilder(cluster, assetBuilder, fileAssets.Assets, encryptionConfigSecretHash)
+	nodeUpAssets, err := nodemodel.BuildNodeUpAssets(ctx, assetBuilder)
+	if err != nil {
+		return nil, err
+	}
+	configBuilder, err := nodemodel.NewNodeUpConfigBuilder(cluster, assetBuilder, encryptionConfigSecretHash)
 	if err != nil {
 		return nil, err
 	}
@@ -521,7 +520,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) (*ApplyResults, error) {
 		KopsModelContext:    modelContext,
 		Lifecycle:           clusterLifecycle,
 		NodeUpConfigBuilder: configBuilder,
-		NodeUpAssets:        fileAssets.NodeUpAssets,
+		NodeUpAssets:        nodeUpAssets.NodeUpAssets,
 	}
 
 	{