From 304476cebfd007ea6496ce286d22423e0e35ac71 Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Thu, 18 Jun 2020 14:04:32 -0700 Subject: [PATCH] Refactor BootstrapScript --- pkg/model/BUILD.bazel | 1 + pkg/model/bootstrapscript.go | 19 +++++---- pkg/model/bootstrapscript_test.go | 14 ++++--- pkg/model/openstackmodel/servergroup_test.go | 15 ++++--- upup/pkg/fi/cloudup/apply_cluster.go | 44 +++++++++++++------- upup/pkg/fi/resources.go | 8 ++++ 6 files changed, 69 insertions(+), 32 deletions(-) diff --git a/pkg/model/BUILD.bazel b/pkg/model/BUILD.bazel index 819fa2bc5a..16b848e762 100644 --- a/pkg/model/BUILD.bazel +++ b/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", diff --git a/pkg/model/bootstrapscript.go b/pkg/model/bootstrapscript.go index bd524b2d84..2906336ae9 100644 --- a/pkg/model/bootstrapscript.go +++ b/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 -func (b *BootstrapScript) KubeEnv(ig *kops.InstanceGroup) (string, error) { - config, err := b.NodeUpConfigBuilder(ig) +// kubeEnv returns the nodeup config for the instance group +func (b *BootstrapScript) kubeEnv(ig *kops.InstanceGroup) (string, error) { + 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) { diff --git a/pkg/model/bootstrapscript_test.go b/pkg/model/bootstrapscript_test.go index b1967bf493..2e86e33607 100644 --- a/pkg/model/bootstrapscript_test.go +++ b/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", diff --git a/pkg/model/openstackmodel/servergroup_test.go b/pkg/model/openstackmodel/servergroup_test.go index c264909d0c..45da8c1267 100644 --- a/pkg/model/openstackmodel/servergroup_test.go +++ b/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) { - return &nodeup.Config{}, nil - }, + NodeUpConfigBuilder: &nodeupConfigBuilder{}, 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) { - return &nodeup.Config{}, nil - }, + NodeUpConfigBuilder: &nodeupConfigBuilder{}, NodeUpSource: map[architectures.Architecture]string{ architectures.ArchitectureAmd64: "source-amd64", architectures.ArchitectureArm64: "source-arm64", diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go index d4671c2096..8aada55179 100644 --- a/upup/pkg/fi/cloudup/apply_cluster.go +++ b/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 -func (c *ApplyClusterCmd) AddFileAssets(assetBuilder *assets.AssetBuilder) error { +// addFileAssets adds the file assets within the assetBuilder +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 { - channels = append(channels, c.Cluster.Spec.Addons[i].Manifest) + for i := range cluster.Spec.Addons { + 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 { diff --git a/upup/pkg/fi/resources.go b/upup/pkg/fi/resources.go index 5133516b91..5d25e55337 100644 --- a/upup/pkg/fi/resources.go +++ b/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