Refactor BootstrapScript

pkg/model/BUILD.bazel

@@ -47,6 +47,7 @@ go_library(
         "//upup/pkg/fi/cloudup/openstack:go_default_library",
         "//upup/pkg/fi/cloudup/openstacktasks:go_default_library",
         "//upup/pkg/fi/fitasks:go_default_library",
+        "//upup/pkg/fi/utils:go_default_library",
         "//util/pkg/architectures:go_default_library",
         "//util/pkg/vfs:go_default_library",
         "//vendor/github.com/blang/semver:go_default_library",

pkg/model/bootstrapscript.go

@@ -29,6 +29,7 @@ import (
 
 	"github.com/ghodss/yaml"
 	"k8s.io/klog"
+	"k8s.io/kops/upup/pkg/fi/utils"
 
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/apis/nodeup"
@@ -38,23 +39,27 @@ import (
 	"k8s.io/kops/util/pkg/architectures"
 )
 
+type NodeUpConfigBuilder interface {
+	BuildConfig(ig *kops.InstanceGroup) (*nodeup.Config, error)
+}
+
 // BootstrapScript creates the bootstrap script
 type BootstrapScript struct {
 	NodeUpSource        map[architectures.Architecture]string
 	NodeUpSourceHash    map[architectures.Architecture]string
-	NodeUpConfigBuilder func(ig *kops.InstanceGroup) (*nodeup.Config, error)
+	NodeUpConfigBuilder NodeUpConfigBuilder
 }
 
-// KubeEnv returns the nodeup config for the instance group
+// kubeEnv returns the nodeup config for the instance group
-func (b *BootstrapScript) KubeEnv(ig *kops.InstanceGroup) (string, error) {
+func (b *BootstrapScript) kubeEnv(ig *kops.InstanceGroup) (string, error) {
-	config, err := b.NodeUpConfigBuilder(ig)
+	config, err := b.NodeUpConfigBuilder.BuildConfig(ig)
 	if err != nil {
 		return "", err
 	}
 
-	data, err := kops.ToRawYaml(config)
+	data, err := utils.YamlMarshal(config)
 	if err != nil {
-		return "", err
+		return "", fmt.Errorf("error converting nodeup config to yaml: %v", err)
 	}
 
 	return string(data), nil
@@ -154,7 +159,7 @@ func (b *BootstrapScript) ResourceNodeUp(ig *kops.InstanceGroup, cluster *kops.C
 			return b.NodeUpSourceHash[architectures.ArchitectureArm64]
 		},
 		"KubeEnv": func() (string, error) {
-			return b.KubeEnv(ig)
+			return b.kubeEnv(ig)
 		},
 
 		"EnvironmentVariables": func() (string, error) {

pkg/model/bootstrapscript_test.go

@@ -53,6 +53,14 @@ func Test_ProxyFunc(t *testing.T) {
 	}
 }
 
+type nodeupConfigBuilder struct {
+	cluster *kops.Cluster
+}
+
+func (n *nodeupConfigBuilder) BuildConfig(ig *kops.InstanceGroup) (*nodeup.Config, error) {
+	return nodeup.NewConfig(n.cluster, ig), nil
+}
+
 func TestBootstrapUserData(t *testing.T) {
 	cs := []struct {
 		Role               kops.InstanceGroupRole
@@ -114,12 +122,8 @@ func TestBootstrapUserData(t *testing.T) {
 		cluster := makeTestCluster(x.HookSpecRoles, x.FileAssetSpecRoles)
 		group := makeTestInstanceGroup(x.Role, x.HookSpecRoles, x.FileAssetSpecRoles)
 
-		renderNodeUpConfig := func(ig *kops.InstanceGroup) (*nodeup.Config, error) {
-			return nodeup.NewConfig(cluster, ig), nil
-		}
-
 		bs := &BootstrapScript{
-			NodeUpConfigBuilder: renderNodeUpConfig,
+			NodeUpConfigBuilder: &nodeupConfigBuilder{cluster: cluster},
 			NodeUpSource: map[architectures.Architecture]string{
 				architectures.ArchitectureAmd64: "NUSourceAmd64",
 				architectures.ArchitectureArm64: "NUSourceArm64",

pkg/model/openstackmodel/servergroup_test.go

@@ -2713,9 +2713,7 @@ func Test_ServerGroupModelBuilder(t *testing.T) {
 		t.Run(testCase.desc, func(t *testing.T) {
 			clusterLifecycle := fi.LifecycleSync
 			bootstrapScriptBuilder := &model.BootstrapScript{
-				NodeUpConfigBuilder: func(ig *kops.InstanceGroup) (*nodeup.Config, error) {
+				NodeUpConfigBuilder: &nodeupConfigBuilder{},
-					return &nodeup.Config{}, nil
-				},
 				NodeUpSource: map[architectures.Architecture]string{
 					architectures.ArchitectureAmd64: "source-amd64",
 					architectures.ArchitectureArm64: "source-arm64",
@@ -3181,11 +3179,16 @@ func compareErrors(t *testing.T, actual, expected error) {
 	}
 }
 
+type nodeupConfigBuilder struct {
+}
+
+func (n *nodeupConfigBuilder) BuildConfig(ig *kops.InstanceGroup) (*nodeup.Config, error) {
+	return &nodeup.Config{}, nil
+}
+
 func mustUserdataForClusterInstance(cluster *kops.Cluster, ig *kops.InstanceGroup) string {
 	bootstrapScriptBuilder := &model.BootstrapScript{
-		NodeUpConfigBuilder: func(ig *kops.InstanceGroup) (*nodeup.Config, error) {
+		NodeUpConfigBuilder: &nodeupConfigBuilder{},
-			return &nodeup.Config{}, nil
-		},
 		NodeUpSource: map[architectures.Architecture]string{
 			architectures.ArchitectureAmd64: "source-amd64",
 			architectures.ArchitectureArm64: "source-arm64",

upup/pkg/fi/cloudup/apply_cluster.go

@@ -305,7 +305,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {
 		}
 	}
 
-	if err := c.AddFileAssets(assetBuilder); err != nil {
+	if err := c.addFileAssets(assetBuilder); err != nil {
 		return err
 	}
 
@@ -684,8 +684,12 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {
 		return secretStore
 	}
 
+	configBuilder, err := c.newNodeUpConfigBuilder(assetBuilder)
+	if err != nil {
+		return err
+	}
 	bootstrapScriptBuilder := &model.BootstrapScript{
-		NodeUpConfigBuilder: func(ig *kops.InstanceGroup) (*nodeup.Config, error) { return c.BuildNodeUpConfig(assetBuilder, ig) },
+		NodeUpConfigBuilder: configBuilder,
 		NodeUpSource:        c.NodeUpSource,
 		NodeUpSourceHash:    c.NodeUpHash,
 	}
@@ -1105,8 +1109,8 @@ func (c *ApplyClusterCmd) validateKubernetesVersion() error {
 	return nil
 }
 
-// AddFileAssets adds the file assets within the assetBuilder
+// addFileAssets adds the file assets within the assetBuilder
-func (c *ApplyClusterCmd) AddFileAssets(assetBuilder *assets.AssetBuilder) error {
+func (c *ApplyClusterCmd) addFileAssets(assetBuilder *assets.AssetBuilder) error {
 
 	var baseURL string
 	var err error
@@ -1240,13 +1244,25 @@ func needsMounterAsset(c *kops.Cluster, instanceGroups []*kops.InstanceGroup) bo
 	}
 }
 
+type nodeUpConfigBuilder struct {
+	*ApplyClusterCmd
+	assetBuilder *assets.AssetBuilder
+}
+
+func (c *ApplyClusterCmd) newNodeUpConfigBuilder(assetBuilder *assets.AssetBuilder) (model.NodeUpConfigBuilder, error) {
+	return &nodeUpConfigBuilder{
+		ApplyClusterCmd: c,
+		assetBuilder:    assetBuilder,
+	}, nil
+}
+
 // BuildNodeUpConfig returns the NodeUp config, in YAML format
-func (c *ApplyClusterCmd) BuildNodeUpConfig(assetBuilder *assets.AssetBuilder, ig *kops.InstanceGroup) (*nodeup.Config, error) {
+func (n *nodeUpConfigBuilder) BuildConfig(ig *kops.InstanceGroup) (*nodeup.Config, error) {
 	if ig == nil {
 		return nil, fmt.Errorf("instanceGroup cannot be nil")
 	}
 
-	cluster := c.Cluster
+	cluster := n.Cluster
 
 	configBase, err := vfs.Context.BuildVfsPath(cluster.Spec.ConfigBase)
 	if err != nil {
@@ -1263,8 +1279,8 @@ func (c *ApplyClusterCmd) BuildNodeUpConfig(assetBuilder *assets.AssetBuilder, i
 		configBase.Join("addons", "bootstrap-channel.yaml").Path(),
 	}
 
-	for i := range c.Cluster.Spec.Addons {
+	for i := range cluster.Spec.Addons {
-		channels = append(channels, c.Cluster.Spec.Addons[i].Manifest)
+		channels = append(channels, cluster.Spec.Addons[i].Manifest)
 	}
 
 	role := ig.Spec.Role
@@ -1283,7 +1299,7 @@ func (c *ApplyClusterCmd) BuildNodeUpConfig(assetBuilder *assets.AssetBuilder, i
 	config.Assets = make(map[architectures.Architecture][]string)
 	for _, arch := range architectures.GetSupprted() {
 		config.Assets[arch] = []string{}
-		for _, a := range c.Assets[arch] {
+		for _, a := range n.Assets[arch] {
 			config.Assets[arch] = append(config.Assets[arch], a.CompactString())
 		}
 	}
@@ -1305,14 +1321,14 @@ func (c *ApplyClusterCmd) BuildNodeUpConfig(assetBuilder *assets.AssetBuilder, i
 
 		for _, arch := range architectures.GetSupprted() {
 			for _, component := range components {
-				baseURL, err := url.Parse(c.Cluster.Spec.KubernetesVersion)
+				baseURL, err := url.Parse(cluster.Spec.KubernetesVersion)
 				if err != nil {
 					return nil, err
 				}
 
 				baseURL.Path = path.Join(baseURL.Path, "/bin/linux", string(arch), component+".tar")
 
-				u, hash, err := assetBuilder.RemapFileAndSHA(baseURL)
+				u, hash, err := n.assetBuilder.RemapFileAndSHA(baseURL)
 				if err != nil {
 					return nil, err
 				}
@@ -1343,7 +1359,7 @@ func (c *ApplyClusterCmd) BuildNodeUpConfig(assetBuilder *assets.AssetBuilder, i
 
 				baseURL.Path = path.Join(baseURL.Path, "/images/"+name+".tar.gz")
 
-				u, hash, err := assetBuilder.RemapFileAndSHA(baseURL)
+				u, hash, err := n.assetBuilder.RemapFileAndSHA(baseURL)
 				if err != nil {
 					return nil, err
 				}
@@ -1358,7 +1374,7 @@ func (c *ApplyClusterCmd) BuildNodeUpConfig(assetBuilder *assets.AssetBuilder, i
 	}
 
 	if isMaster || useGossip {
-		u, hash, err := ProtokubeImageSource(assetBuilder)
+		u, hash, err := ProtokubeImageSource(n.assetBuilder)
 		if err != nil {
 			return nil, err
 		}
@@ -1381,7 +1397,7 @@ func (c *ApplyClusterCmd) BuildNodeUpConfig(assetBuilder *assets.AssetBuilder, i
 		}
 	}
 
-	for _, manifest := range assetBuilder.StaticManifests {
+	for _, manifest := range n.assetBuilder.StaticManifests {
 		match := false
 		for _, r := range manifest.Roles {
 			if r == role {

upup/pkg/fi/resources.go

@@ -207,6 +207,7 @@ type ResourceHolder struct {
 }
 
 var _ Resource = &ResourceHolder{}
+var _ HasDependencies = &ResourceHolder{}
 
 // Open implements the Open method of the Resource interface
 func (o *ResourceHolder) Open() (io.Reader, error) {
@@ -216,6 +217,13 @@ func (o *ResourceHolder) Open() (io.Reader, error) {
 	return o.Resource.Open()
 }
 
+func (r *ResourceHolder) GetDependencies(tasks map[string]Task) []Task {
+	if hasDependencies, ok := r.Resource.(HasDependencies); ok {
+		return hasDependencies.GetDependencies(tasks)
+	}
+	return nil
+}
+
 // UnmarshalJSON implements the special JSON marshaling for the resource, rendering the name
 func (o *ResourceHolder) UnmarshalJSON(data []byte) error {
 	var jsonName string