Repository: kubernetes/kops (go)

Issue:
Disabling kubeProxy and enabling cilium nodeport before cilium is first installed seems to break the cluster
**1. What `kops` version are you running? The command `kops version`, will display
 this information.**

``Version 1.18.0-beta.1 (git-ec8022b352)``

**2. What Kubernetes version are you running? `kubectl version` will print the
 version if a cluster is running or provide the Kubernetes version specified as
 a `kops` flag.**

```
Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.4", GitCommit:"c96aede7b5205121079932896c4ad89bb93260af", GitTreeState:"clean", BuildDate:"2020-06-17T11:41:22Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.3", GitCommit:"2e7996e3e2712684bc73f0dec0200d64eec7fe40", GitTreeState:"clean", BuildDate:"2020-05-20T12:43:34Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}
```

**3. What cloud provider are you using?**

AWS

**4. What commands did you run?  What is the simplest way to reproduce this issue?**
**5. What happened after the commands executed?**

After fixing the etcd cluster for cilium issue, i put cilium back in the spec with nodeport set to true and kubeProxy set to false. What happened is kubeProxy was removed from the first master and the process stopped, and the first master was stuck in state "NotReady."

I then changed the nodeport setting to false, re enabled kubeProxy, which started to move forward with the cilium install, but then i experienced another issue which i am filing another bug report for.

**6. What did you expect to happen?**

Expected cilium to deploy then cut over from the old way.

**7. Please provide your cluster manifest. Execute
  `kops get --name my.example.com -o yaml` to display your cluster manifest.
  You may want to remove your cluster name and other sensitive information.**

```
apiVersion: kops.k8s.io/v1alpha2
kind: Cluster
metadata:
  creationTimestamp: "2019-05-29T15:39:14Z"
  generation: 24
  name: clustername
spec:
  additionalPolicies:
    master: |
      [
        {
          "Effect": "Allow",
          "Action": [
            "route53:ChangeResourceRecordSets"
          ],
          "Resource": [
            "arn:aws:route53:::hostedzone/ZONE",
          ]
        },
        {
          "Effect": "Allow",
          "Action": [
            "acm:DescribeCertificate",
            "acm:ListCertificates",
            "acm:GetCertificate",
            "cognito-idp:DescribeUserPoolClient",
            "ec2:AuthorizeSecurityGroupIngress",
            "ec2:CreateSecurityGroup",
            "ec2:CreateTags",
            "ec2:DeleteTags",
            "ec2:DeleteSecurityGroup",
            "ec2:DescribeAccountAttributes",
            "ec2:DescribeAddresses",
            "ec2:DescribeInstances",
            "ec2:DescribeInstanceStatus",
            "ec2:DescribeInternetGateways",
            "ec2:DescribeNetworkInterfaces",
            "ec2:DescribeSecurityGroups",
            "ec2:DescribeSubnets",
            "ec2:DescribeTags",
            "ec2:DescribeVpcs",
            "ec2:ModifyInstanceAttribute",
            "ec2:ModifyNetworkInterfaceAttribute",
            "ec2:RevokeSecurityGroupIngress",
            "elasticloadbalancing:AddListenerCertificates",
            "elasticloadbalancing:AddTags",
            "elasticloadbalancing:CreateListener",
            "elasticloadbalancing:CreateLoadBalancer",
            "elasticloadbalancing:CreateRule",
            "elasticloadbalancing:CreateTargetGroup",
            "elasticloadbalancing:DeleteListener",
            "elasticloadbalancing:DeleteLoadBalancer",
            "elasticloadbalancing:DeleteRule",
            "elasticloadbalancing:DeleteTargetGroup",
            "elasticloadbalancing:DeregisterTargets",
            "elasticloadbalancing:DescribeListenerCertificates",
            "elasticloadbalancing:DescribeListeners",
            "elasticloadbalancing:DescribeLoadBalancers",
            "elasticloadbalancing:DescribeLoadBalancerAttributes",
            "elasticloadbalancing:DescribeRules",
            "elasticloadbalancing:DescribeSSLPolicies",
            "elasticloadbalancing:DescribeTags",
            "elasticloadbalancing:DescribeTargetGroups",
            "elasticloadbalancing:DescribeTargetGroupAttributes",
            "elasticloadbalancing:DescribeTargetHealth",
            "elasticloadbalancing:ModifyListener",
            "elasticloadbalancing:ModifyLoadBalancerAttributes",
            "elasticloadbalancing:ModifyRule",
            "elasticloadbalancing:ModifyTargetGroup",
            "elasticloadbalancing:ModifyTargetGroupAttributes",
            "elasticloadbalancing:RegisterTargets",
            "elasticloadbalancing:RemoveListenerCertificates",
            "elasticloadbalancing:RemoveTags",
            "elasticloadbalancing:SetIpAddressType",
            "elasticloadbalancing:SetSecurityGroups",
            "elasticloadbalancing:SetSubnets",
            "elasticloadbalancing:SetWebACL",
            "iam:CreateServiceLinkedRole",
            "iam:GetServerCertificate",
            "iam:ListServerCertificates",
            "route53:ListHostedZones",
            "route53:ListResourceRecordSets",
            "tag:GetResources",
            "tag:TagResources",
            "waf:GetWebACL",
            "waf-regional:GetWebACLForResource",
            "waf-regional:GetWebACL",
            "waf-regional:AssociateWebACL",
            "waf-regional:DisassociateWebACL"
          ],
          "Resource": [
            "*"
          ]
        }
      ]
    node: |
      [
        {
          "Effect": "Allow",
          "Action": [
            "s3:ListBucket",
            "s3:GetObject"
          ],
          "Resource": [
            "bucket"
          ]
        },
        {
          "Effect": "Allow",
          "Action": [
            "route53:ChangeResourceRecordSets"
          ],
          "Resource": [
            "arn:aws:route53:::hostedzone/ZONE",
          ]
        },
        {
          "Effect": "Allow",
          "Action": [
            "acm:DescribeCertificate",
            "acm:ListCertificates",
            "acm:GetCertificate",
            "cognito-idp:DescribeUserPoolClient",
            "ec2:AuthorizeSecurityGroupIngress",
            "ec2:CreateSecurityGroup",
            "ec2:CreateTags",
            "ec2:DeleteTags",
            "ec2:DeleteSecurityGroup",
            "ec2:DescribeAccountAttributes",
            "ec2:DescribeAddresses",
            "ec2:DescribeInstances",
            "ec2:DescribeInstanceStatus",
            "ec2:DescribeInternetGateways",
            "ec2:DescribeNetworkInterfaces",
            "ec2:DescribeSecurityGroups",
            "ec2:DescribeSubnets",
            "ec2:DescribeTags",
            "ec2:DescribeVpcs",
            "ec2:ModifyInstanceAttribute",
            "ec2:ModifyNetworkInterfaceAttribute",
            "ec2:RevokeSecurityGroupIngress",
            "elasticloadbalancing:AddListenerCertificates",
            "elasticloadbalancing:AddTags",
            "elasticloadbalancing:CreateListener",
            "elasticloadbalancing:CreateLoadBalancer",
            "elasticloadbalancing:CreateRule",
            "elasticloadbalancing:CreateTargetGroup",
            "elasticloadbalancing:DeleteListener",
            "elasticloadbalancing:DeleteLoadBalancer",
            "elasticloadbalancing:DeleteRule",
            "elasticloadbalancing:DeleteTargetGroup",
            "elasticloadbalancing:DeregisterTargets",
            "elasticloadbalancing:DescribeListenerCertificates",
            "elasticloadbalancing:DescribeListeners",
            "elasticloadbalancing:DescribeLoadBalancers",
            "elasticloadbalancing:DescribeLoadBalancerAttributes",
            "elasticloadbalancing:DescribeRules",
            "elasticloadbalancing:DescribeSSLPolicies",
            "elasticloadbalancing:DescribeTags",
            "elasticloadbalancing:DescribeTargetGroups",
            "elasticloadbalancing:DescribeTargetGroupAttributes",
            "elasticloadbalancing:DescribeTargetHealth",
            "elasticloadbalancing:ModifyListener",
            "elasticloadbalancing:ModifyLoadBalancerAttributes",
            "elasticloadbalancing:ModifyRule",
            "elasticloadbalancing:ModifyTargetGroup",
            "elasticloadbalancing:ModifyTargetGroupAttributes",
            "elasticloadbalancing:RegisterTargets",
            "elasticloadbalancing:RemoveListenerCertificates",
            "elasticloadbalancing:RemoveTags",
            "elasticloadbalancing:SetIpAddressType",
            "elasticloadbalancing:SetSecurityGroups",
            "elasticloadbalancing:SetSubnets",
            "elasticloadbalancing:SetWebACL",
            "iam:CreateServiceLinkedRole",
            "iam:GetServerCertificate",
            "iam:ListServerCertificates",
            "route53:ListHostedZones",
            "route53:ListResourceRecordSets",
            "tag:GetResources",
            "tag:TagResources",
            "waf:GetWebACL",
            "waf-regional:GetWebACLForResource",
            "waf-regional:GetWebACL",
            "waf-regional:AssociateWebACL",
            "waf-regional:DisassociateWebACL"
          ],
          "Resource": [
            "*"
          ]
        }
      ]
  api:
    loadBalancer:
      type: Public
  authorization:
    rbac: {}
  channel: stable
  cloudProvider: aws
  configBase: s3://store
  etcdClusters:
  - cpuRequest: 500m
    etcdMembers:
    - instanceGroup: master-us-east-1a
      name: a
    - instanceGroup: master-us-east-1b
      name: b
    - instanceGroup: master-us-east-1d
      name: d
    memoryRequest: 1000Mi
    name: main
  - cpuRequest: 500m
    etcdMembers:
    - instanceGroup: master-us-east-1a
      name: a
    - instanceGroup: master-us-east-1b
      name: b
    - instanceGroup: master-us-east-1d
      name: d
    memoryRequest: 1000Mi
    name: events
  - cpuRequest: 500m
    etcdMembers:
    - instanceGroup: master-us-east-1a
      name: a
    - instanceGroup: master-us-east-1b
      name: b
    - instanceGroup: master-us-east-1d
      name: d
    memoryRequest: 1000Mi
    name: cilium
  iam:
    allowContainerRegistry: true
    legacy: false
  kubeDNS:
    provider: CoreDNS
  kubelet:
    anonymousAuth: false
    authenticationTokenWebhook: true
    authorizationMode: Webhook
  kubernetesApiAccess:
  - x.x.x.x
  kubernetesVersion: 1.18.3
  masterInternalName: api.internal.clustername
  masterPublicName: api.clustername
  networkCIDR: 172.20.0.0/16
  kubeProxy:
    enabled: false
  networking:
    cilium:
      IPTablesRulesNoinstall: false
      autoDirectNodeRoutes: true
      bpfCTGlobalAnyMax: 262144
      bpfCTGlobalTCPMax: 524288
      clusterName: clustername
      cniBinPath: ""
      enableNodePort: true
      enableRemoteNodeIdentity: true
      enableipv4: false
      enableipv6: false
      etcdManaged: true
      monitorAggregation: medium
      nodeInitBootstrapFile: ""
      preallocateBPFMaps: true
      reconfigureKubelet: false
      removeCbrBridge: false
      restartPods: false
      sidecarIstioProxyImage: ""
      toFqdnsEnablePoller: false
  nonMasqueradeCIDR: 100.64.0.0/10
  sshAccess:
  - x.x.x.x
  subnets:
  - cidr: 172.20.32.0/19
    name: us-east-1a
    type: Private
    zone: us-east-1a
  - cidr: 172.20.64.0/19
    name: us-east-1b
    type: Private
    zone: us-east-1b
  - cidr: 172.20.96.0/19
    name: us-east-1d
    type: Private
    zone: us-east-1d
  - cidr: 172.20.0.0/22
    name: utility-us-east-1a
    type: Utility
    zone: us-east-1a
  - cidr: 172.20.4.0/22
    name: utility-us-east-1b
    type: Utility
    zone: us-east-1b
  - cidr: 172.20.8.0/22
    name: utility-us-east-1d
    type: Utility
    zone: us-east-1d
  topology:
    dns:
      type: Public
    masters: private
    nodes: private
---
apiVersion: kops.k8s.io/v1alpha2
kind: InstanceGroup
metadata:
  creationTimestamp: "2020-04-21T15:03:19Z"
  generation: 10
  labels:
    kops.k8s.io/cluster: clustername
  name: elasticsearch
spec:
  image: ubuntu/ubuntu/images/hvm-ssd/ubuntu-focal-20.04-amd64-server-20200609
  machineType: t3.2xlarge
  maxSize: 3
  minSize: 3
  nodeLabels:
    kops.k8s.io/instancegroup: elasticsearch
    project: kuato
  role: Node
  rootVolumeSize: 1000
  subnets:
  - us-east-1a
  - us-east-1b
  - us-east-1d
  taints:
  - project=kuato:NoSchedule

---

apiVersion: kops.k8s.io/v1alpha2
kind: InstanceGroup
metadata:
  creationTimestamp: "2020-05-21T12:37:05Z"
  generation: 5
  labels:
    kops.k8s.io/cluster: clustername
  name: krang
spec:
  image: ubuntu/ubuntu/images/hvm-ssd/ubuntu-focal-20.04-amd64-server-20200609
  machineType: r5a.xlarge
  maxSize: 3
  minSize: 3
  nodeLabels:
    kops.k8s.io/instancegroup: krang
    project: krang
  role: Node
  subnets:
  - us-east-1a
  - us-east-1b
  - us-east-1d
  taints:
  - project=krang:NoSchedule

---

apiVersion: kops.k8s.io/v1alpha2
kind: InstanceGroup
metadata:
  creationTimestamp: "2019-05-29T15:39:14Z"
  generation: 6
  labels:
    kops.k8s.io/cluster: clustername
  name: master-us-east-1a
spec:
  image: ubuntu/ubuntu/images/hvm-ssd/ubuntu-focal-20.04-amd64-server-20200609
  machineType: t3a.xlarge
  maxSize: 1
  minSize: 1
  nodeLabels:
    kops.k8s.io/instancegroup: master-us-east-1a
  role: Master
  subnets:
  - us-east-1a

---

apiVersion: kops.k8s.io/v1alpha2
kind: InstanceGroup
metadata:
  creationTimestamp: "2019-05-29T15:39:14Z"
  generation: 6
  labels:
    kops.k8s.io/cluster: clustername
  name: master-us-east-1b
spec:
  image: ubuntu/ubuntu/images/hvm-ssd/ubuntu-focal-20.04-amd64-server-20200609
  machineType: t3a.xlarge
  maxSize: 1
  minSize: 1
  nodeLabels:
    kops.k8s.io/instancegroup: master-us-east-1b
  role: Master
  subnets:
  - us-east-1b

---

apiVersion: kops.k8s.io/v1alpha2
kind: InstanceGroup
metadata:
  creationTimestamp: "2019-05-29T15:39:14Z"
  generation: 6
  labels:
    kops.k8s.io/cluster: clustername
  name: master-us-east-1d
spec:
  image: ubuntu/ubuntu/images/hvm-ssd/ubuntu-focal-20.04-amd64-server-20200609
  machineType: t3a.xlarge
  maxSize: 1
  minSize: 1
  nodeLabels:
    kops.k8s.io/instancegroup: master-us-east-1d
  role: Master
  subnets:
  - us-east-1d

---

apiVersion: kops.k8s.io/v1alpha2
kind: InstanceGroup
metadata:
  creationTimestamp: "2019-05-29T15:39:14Z"
  generation: 20
  labels:
    kops.k8s.io/cluster: clustername
  name: nodes
spec:
  image: ubuntu/ubuntu/images/hvm-ssd/ubuntu-focal-20.04-amd64-server-20200609
  machineType: m5a.2xlarge
  maxSize: 12
  minSize: 12
  nodeLabels:
    kops.k8s.io/instancegroup: nodes
  role: Node
  subnets:
  - us-east-1a
  - us-east-1b
  - us-east-1d
```

**8. Please run the commands with most verbose logging by adding the `-v 10` flag.
  Paste the logs into this report, or in a gist and provide the gist link here.**


**9. Anything else do we need to know?**

Another bug report incoming

Pull request description:
Cilium requires manual restart when migrating to nodeport
Fixes #9447 

Patch:
diff --git a/docs/networking/cilium.md b/docs/networking/cilium.md
index 93e0393903..8e6b85ee25 100644
--- a/docs/networking/cilium.md
+++ b/docs/networking/cilium.md
@@ -68,6 +68,15 @@ Be aware that you need to use an AMI with at least Linux 4.19.57 for this featur
       enableNodePort: true
 ```
 
+If you are migrating an existing cluster, you need to manually roll the cilium DaemonSet before rolling the cluster:
+
+```
+kops update cluster
+kops update cluster --yes
+kubectl rollout restart ds/cilium -n kube-system
+kops rolling-update cluster --yes
+```
+
 ### Enabling Cilium ENI IPAM
 
 As of Kops 1.18, you can have Cilium provision AWS managed adresses and attach them directly to Pods much like Lyft VPC and AWS VPC. See [the Cilium docs for more information](https://docs.cilium.io/en/v1.6/concepts/ipam/eni/)
diff --git a/pkg/model/BUILD.bazel b/pkg/model/BUILD.bazel
index 16b848e762..819fa2bc5a 100644
--- a/pkg/model/BUILD.bazel
+++ b/pkg/model/BUILD.bazel
@@ -47,7 +47,6 @@ 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 2906336ae9..bd524b2d84 100644
--- a/pkg/model/bootstrapscript.go
+++ b/pkg/model/bootstrapscript.go
@@ -29,7 +29,6 @@ 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"
@@ -39,27 +38,23 @@ 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 NodeUpConfigBuilder
+	NodeUpConfigBuilder func(ig *kops.InstanceGroup) (*nodeup.Config, error)
 }
 
-// kubeEnv returns the nodeup config for the instance group
-func (b *BootstrapScript) kubeEnv(ig *kops.InstanceGroup) (string, error) {
-	config, err := b.NodeUpConfigBuilder.BuildConfig(ig)
+// KubeEnv returns the nodeup config for the instance group
+func (b *BootstrapScript) KubeEnv(ig *kops.InstanceGroup) (string, error) {
+	config, err := b.NodeUpConfigBuilder(ig)
 	if err != nil {
 		return "", err
 	}
 
-	data, err := utils.YamlMarshal(config)
+	data, err := kops.ToRawYaml(config)
 	if err != nil {
-		return "", fmt.Errorf("error converting nodeup config to yaml: %v", err)
+		return "", err
 	}
 
 	return string(data), nil
@@ -159,7 +154,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/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go
index cc97b37b3a..f77c319ef0 100644
--- a/upup/pkg/fi/cloudup/apply_cluster.go
+++ b/upup/pkg/fi/cloudup/apply_cluster.go
@@ -318,7 +318,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
 	}
 
@@ -697,12 +697,8 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {
 		return secretStore
 	}
 
-	configBuilder, err := c.newNodeUpConfigBuilder(assetBuilder)
-	if err != nil {
-		return err
-	}
 	bootstrapScriptBuilder := &model.BootstrapScript{
-		NodeUpConfigBuilder: configBuilder,
+		NodeUpConfigBuilder: func(ig *kops.InstanceGroup) (*nodeup.Config, error) { return c.BuildNodeUpConfig(assetBuilder, ig) },
 		NodeUpSource:        c.NodeUpSource,
 		NodeUpSourceHash:    c.NodeUpHash,
 	}
@@ -1122,8 +1118,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
@@ -1257,19 +1253,12 @@ func needsMounterAsset(c *kops.Cluster, instanceGroups []*kops.InstanceGroup) bo
 	}
 }
 
-type nodeUpConfigBuilder struct {
-	*ApplyClusterCmd
-	assetBuilder   *assets.AssetBuilder
-	channels       []string
-	configBase     vfs.Path
-	cluster        *kops.Cluster
-	etcdManifests  map[kops.InstanceGroupRole][]string
-	images         map[kops.InstanceGroupRole]map[architectures.Architecture][]*nodeup.Image
-	nodeUpTags     []string
-	protokubeImage map[kops.InstanceGroupRole]*nodeup.Image
-}
+// BuildNodeUpConfig returns the NodeUp config, in YAML format
+func (c *ApplyClusterCmd) BuildNodeUpConfig(assetBuilder *assets.AssetBuilder, ig *kops.InstanceGroup) (*nodeup.Config, error) {
+	if ig == nil {
+		return nil, fmt.Errorf("instanceGroup cannot be nil")
+	}
 
-func (c *ApplyClusterCmd) newNodeUpConfigBuilder(assetBuilder *assets.AssetBuilder) (model.NodeUpConfigBuilder, error) {
 	cluster := c.Cluster
 
 	configBase, err := vfs.Context.BuildVfsPath(cluster.Spec.ConfigBase)
@@ -1287,153 +1276,125 @@ func (c *ApplyClusterCmd) newNodeUpConfigBuilder(assetBuilder *assets.AssetBuild
 		configBase.Join("addons", "bootstrap-channel.yaml").Path(),
 	}
 
-	for i := range cluster.Spec.Addons {
-		channels = append(channels, cluster.Spec.Addons[i].Manifest)
+	for i := range c.Cluster.Spec.Addons {
+		channels = append(channels, c.Cluster.Spec.Addons[i].Manifest)
 	}
 
-	nodeUpTags, err := buildNodeupTags(cluster, clusterTags)
+	role := ig.Spec.Role
+	if role == "" {
+		return nil, fmt.Errorf("cannot determine role for instance group: %v", ig.ObjectMeta.Name)
+	}
+
+	nodeUpTags, err := buildNodeupTags(role, cluster, clusterTags)
 	if err != nil {
 		return nil, err
 	}
 
-	useGossip := dns.IsGossipHostname(cluster.Spec.MasterInternalName)
-
-	etcdManifests := map[kops.InstanceGroupRole][]string{}
-	images := map[kops.InstanceGroupRole]map[architectures.Architecture][]*nodeup.Image{}
-	protokubeImage := map[kops.InstanceGroupRole]*nodeup.Image{}
+	config := nodeup.NewConfig(cluster, ig)
+	config.Tags = append(config.Tags, nodeUpTags.List()...)
 
-	for _, role := range kops.AllInstanceGroupRoles {
-		isMaster := role == kops.InstanceGroupRoleMaster
+	config.Assets = make(map[architectures.Architecture][]string)
+	for _, arch := range architectures.GetSupprted() {
+		config.Assets[arch] = []string{}
+		for _, a := range c.Assets[arch] {
+			config.Assets[arch] = append(config.Assets[arch], a.CompactString())
+		}
+	}
 
-		images[role] = make(map[architectures.Architecture][]*nodeup.Image)
-		if components.IsBaseURL(cluster.Spec.KubernetesVersion) {
-			// When using a custom version, we want to preload the images over http
-			components := []string{"kube-proxy"}
-			if isMaster {
-				components = append(components, "kube-apiserver", "kube-controller-manager", "kube-scheduler")
-			}
+	config.ClusterName = cluster.ObjectMeta.Name
+	config.ConfigBase = fi.String(configBase.Path())
+	config.InstanceGroupName = ig.ObjectMeta.Name
 
-			for _, arch := range architectures.GetSupprted() {
-				for _, component := range components {
-					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)
-					if err != nil {
-						return nil, err
-					}
-
-					image := &nodeup.Image{
-						Sources: []string{u.String()},
-						Hash:    hash.Hex(),
-					}
-					images[role][arch] = append(images[role][arch], image)
-				}
-			}
+	useGossip := dns.IsGossipHostname(cluster.Spec.MasterInternalName)
+	isMaster := role == kops.InstanceGroupRoleMaster
+
+	images := make(map[architectures.Architecture][]*nodeup.Image)
+	if components.IsBaseURL(cluster.Spec.KubernetesVersion) {
+		// When using a custom version, we want to preload the images over http
+		components := []string{"kube-proxy"}
+		if isMaster {
+			components = append(components, "kube-apiserver", "kube-controller-manager", "kube-scheduler")
 		}
 
-		// `docker load` our images when using a KOPS_BASE_URL, so we
-		// don't need to push/pull from a registry
-		if os.Getenv("KOPS_BASE_URL") != "" && isMaster {
-			for _, arch := range architectures.GetSupprted() {
-				// TODO: Build multi-arch Kops images
-				if arch != architectures.ArchitectureAmd64 {
-					continue
+		for _, arch := range architectures.GetSupprted() {
+			for _, component := range components {
+				baseURL, err := url.Parse(c.Cluster.Spec.KubernetesVersion)
+				if err != nil {
+					return nil, err
 				}
 
-				for _, name := range []string{"kops-controller", "dns-controller", "kube-apiserver-healthcheck"} {
-					baseURL, err := url.Parse(os.Getenv("KOPS_BASE_URL"))
-					if err != nil {
-						return nil, err
-					}
-
-					baseURL.Path = path.Join(baseURL.Path, "/images/"+name+".tar.gz")
+				baseURL.Path = path.Join(baseURL.Path, "/bin/linux", string(arch), component+".tar")
 
-					u, hash, err := assetBuilder.RemapFileAndSHA(baseURL)
-					if err != nil {
-						return nil, err
-					}
+				u, hash, err := assetBuilder.RemapFileAndSHA(baseURL)
+				if err != nil {
+					return nil, err
+				}
 
-					image := &nodeup.Image{
-						Sources: []string{u.String()},
-						Hash:    hash.Hex(),
-					}
-					images[role][arch] = append(images[role][arch], image)
+				image := &nodeup.Image{
+					Sources: []string{u.String()},
+					Hash:    hash.Hex(),
 				}
+				images[arch] = append(images[arch], image)
 			}
 		}
+	}
 
-		if isMaster || useGossip {
-			u, hash, err := ProtokubeImageSource(assetBuilder)
-			if err != nil {
-				return nil, err
+	// `docker load` our images when using a KOPS_BASE_URL, so we
+	// don't need to push/pull from a registry
+	if os.Getenv("KOPS_BASE_URL") != "" && isMaster {
+		for _, arch := range architectures.GetSupprted() {
+			// TODO: Build multi-arch Kops images
+			if arch != architectures.ArchitectureAmd64 {
+				continue
 			}
 
-			asset := BuildMirroredAsset(u, hash)
+			for _, name := range []string{"kops-controller", "dns-controller", "kube-apiserver-healthcheck"} {
+				baseURL, err := url.Parse(os.Getenv("KOPS_BASE_URL"))
+				if err != nil {
+					return nil, err
+				}
+
+				baseURL.Path = path.Join(baseURL.Path, "/images/"+name+".tar.gz")
 
-			protokubeImage[role] = &nodeup.Image{
-				Name:    kopsbase.DefaultProtokubeImageName(),
-				Sources: asset.Locations,
-				Hash:    asset.Hash.Hex(),
-			}
-		}
+				u, hash, err := assetBuilder.RemapFileAndSHA(baseURL)
+				if err != nil {
+					return nil, err
+				}
 
-		if role == kops.InstanceGroupRoleMaster {
-			for _, etcdCluster := range cluster.Spec.EtcdClusters {
-				if etcdCluster.Provider == kops.EtcdProviderTypeManager {
-					p := configBase.Join("manifests/etcd/" + etcdCluster.Name + ".yaml").Path()
-					etcdManifests[role] = append(etcdManifests[role], p)
+				image := &nodeup.Image{
+					Sources: []string{u.String()},
+					Hash:    hash.Hex(),
 				}
+				images[arch] = append(images[arch], image)
 			}
 		}
 	}
 
-	configBuilder := nodeUpConfigBuilder{
-		ApplyClusterCmd: c,
-		assetBuilder:    assetBuilder,
-		channels:        channels,
-		configBase:      configBase,
-		cluster:         cluster,
-		etcdManifests:   etcdManifests,
-		images:          images,
-		nodeUpTags:      nodeUpTags.List(),
-		protokubeImage:  protokubeImage,
-	}
-	return &configBuilder, nil
-}
+	if isMaster || useGossip {
+		u, hash, err := ProtokubeImageSource(assetBuilder)
+		if err != nil {
+			return nil, err
+		}
 
-// BuildNodeUpConfig returns the NodeUp config, in YAML format
-func (n *nodeUpConfigBuilder) BuildConfig(ig *kops.InstanceGroup) (*nodeup.Config, error) {
-	cluster := n.cluster
+		asset := BuildMirroredAsset(u, hash)
 
-	if ig == nil {
-		return nil, fmt.Errorf("instanceGroup cannot be nil")
-	}
-
-	role := ig.Spec.Role
-	if role == "" {
-		return nil, fmt.Errorf("cannot determine role for instance group: %v", ig.ObjectMeta.Name)
+		config.ProtokubeImage = &nodeup.Image{
+			Name:    kopsbase.DefaultProtokubeImageName(),
+			Sources: asset.Locations,
+			Hash:    asset.Hash.Hex(),
+		}
 	}
 
-	config := nodeup.NewConfig(cluster, ig)
-	config.Tags = append(config.Tags, n.nodeUpTags...)
-
-	config.Assets = make(map[architectures.Architecture][]string)
-	for _, arch := range architectures.GetSupprted() {
-		config.Assets[arch] = []string{}
-		for _, a := range n.Assets[arch] {
-			config.Assets[arch] = append(config.Assets[arch], a.CompactString())
+	if role == kops.InstanceGroupRoleMaster {
+		for _, etcdCluster := range cluster.Spec.EtcdClusters {
+			if etcdCluster.Provider == kops.EtcdProviderTypeManager {
+				p := configBase.Join("manifests/etcd/" + etcdCluster.Name + ".yaml").Path()
+				config.EtcdManifests = append(config.EtcdManifests, p)
+			}
 		}
 	}
-	config.ClusterName = cluster.ObjectMeta.Name
-	config.ConfigBase = fi.String(n.configBase.Path())
-	config.InstanceGroupName = ig.ObjectMeta.Name
 
-	for _, manifest := range n.assetBuilder.StaticManifests {
+	for _, manifest := range assetBuilder.StaticManifests {
 		match := false
 		for _, r := range manifest.Roles {
 			if r == role {
@@ -1451,10 +1412,8 @@ func (n *nodeUpConfigBuilder) BuildConfig(ig *kops.InstanceGroup) (*nodeup.Confi
 		})
 	}
 
-	config.Images = n.images[role]
-	config.Channels = n.channels
-	config.EtcdManifests = n.etcdManifests[role]
-	config.ProtokubeImage = n.protokubeImage[role]
+	config.Images = images
+	config.Channels = channels
 
 	return config, nil
 }
diff --git a/upup/pkg/fi/cloudup/tagbuilder.go b/upup/pkg/fi/cloudup/tagbuilder.go
index 02bd1ab17e..d0895e3ba9 100644
--- a/upup/pkg/fi/cloudup/tagbuilder.go
+++ b/upup/pkg/fi/cloudup/tagbuilder.go
@@ -67,7 +67,7 @@ func buildCloudupTags(cluster *api.Cluster) (sets.String, error) {
 	return tags, nil
 }
 
-func buildNodeupTags(cluster *api.Cluster, clusterTags sets.String) (sets.String, error) {
+func buildNodeupTags(role api.InstanceGroupRole, cluster *api.Cluster, clusterTags sets.String) (sets.String, error) {
 	tags := sets.NewString()
 
 	networking := cluster.Spec.Networking
diff --git a/upup/pkg/fi/resources.go b/upup/pkg/fi/resources.go
index 5d25e55337..5133516b91 100644
--- a/upup/pkg/fi/resources.go
+++ b/upup/pkg/fi/resources.go
@@ -207,7 +207,6 @@ 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) {
@@ -217,13 +216,6 @@ 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


Test patch:
diff --git a/pkg/model/bootstrapscript_test.go b/pkg/model/bootstrapscript_test.go
index 2e86e33607..b1967bf493 100644
--- a/pkg/model/bootstrapscript_test.go
+++ b/pkg/model/bootstrapscript_test.go
@@ -53,14 +53,6 @@ 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
@@ -122,8 +114,12 @@ 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: &nodeupConfigBuilder{cluster: cluster},
+			NodeUpConfigBuilder: renderNodeUpConfig,
 			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 45da8c1267..c264909d0c 100644
--- a/pkg/model/openstackmodel/servergroup_test.go
+++ b/pkg/model/openstackmodel/servergroup_test.go
@@ -2713,7 +2713,9 @@ func Test_ServerGroupModelBuilder(t *testing.T) {
 		t.Run(testCase.desc, func(t *testing.T) {
 			clusterLifecycle := fi.LifecycleSync
 			bootstrapScriptBuilder := &model.BootstrapScript{
-				NodeUpConfigBuilder: &nodeupConfigBuilder{},
+				NodeUpConfigBuilder: func(ig *kops.InstanceGroup) (*nodeup.Config, error) {
+					return &nodeup.Config{}, nil
+				},
 				NodeUpSource: map[architectures.Architecture]string{
 					architectures.ArchitectureAmd64: "source-amd64",
 					architectures.ArchitectureArm64: "source-arm64",
@@ -3179,16 +3181,11 @@ 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: &nodeupConfigBuilder{},
+		NodeUpConfigBuilder: func(ig *kops.InstanceGroup) (*nodeup.Config, error) {
+			return &nodeup.Config{}, nil
+		},
 		NodeUpSource: map[architectures.Architecture]string{
 			architectures.ArchitectureAmd64: "source-amd64",
 			architectures.ArchitectureArm64: "source-arm64",
diff --git a/upup/pkg/fi/cloudup/tagbuilder_test.go b/upup/pkg/fi/cloudup/tagbuilder_test.go
index f851c69c1c..7326d50661 100644
--- a/upup/pkg/fi/cloudup/tagbuilder_test.go
+++ b/upup/pkg/fi/cloudup/tagbuilder_test.go
@@ -79,7 +79,7 @@ func TestBuildTags_CloudProvider_AWS_Weave(t *testing.T) {
 		t.Fatal("tag _aws not found")
 	}
 
-	nodeUpTags, err := buildNodeupTags(c, tags)
+	nodeUpTags, err := buildNodeupTags(api.InstanceGroupRoleNode, c, tags)
 	if err != nil {
 		t.Fatalf("buildNodeupTags error: %v", err)
 	}
@@ -105,7 +105,7 @@ func TestBuildTags_CloudProvider_AWS_Flannel(t *testing.T) {
 		t.Fatal("tag _aws not found")
 	}
 
-	nodeUpTags, err := buildNodeupTags(c, tags)
+	nodeUpTags, err := buildNodeupTags(api.InstanceGroupRoleNode, c, tags)
 	if err != nil {
 		t.Fatalf("buildNodeupTags error: %v", err)
 	}
@@ -131,7 +131,7 @@ func TestBuildTags_CloudProvider_AWS_Calico(t *testing.T) {
 		t.Fatal("tag _aws not found")
 	}
 
-	nodeUpTags, err := buildNodeupTags(c, tags)
+	nodeUpTags, err := buildNodeupTags(api.InstanceGroupRoleNode, c, tags)
 	if err != nil {
 		t.Fatalf("buildNodeupTags error: %v", err)
 	}
@@ -157,7 +157,7 @@ func TestBuildTags_CloudProvider_AWS_Canal(t *testing.T) {
 		t.Fatal("tag _aws not found")
 	}
 
-	nodeUpTags, err := buildNodeupTags(c, tags)
+	nodeUpTags, err := buildNodeupTags(api.InstanceGroupRoleNode, c, tags)
 	if err != nil {
 		t.Fatalf("buildNodeupTags error: %v", err)
 	}
@@ -180,7 +180,7 @@ func TestBuildTags_CloudProvider_AWS(t *testing.T) {
 		t.Fatal("tag _aws not found")
 	}
 
-	nodeUpTags, err := buildNodeupTags(c, tags)
+	nodeUpTags, err := buildNodeupTags(api.InstanceGroupRoleNode, c, tags)
 	if err != nil {
 		t.Fatalf("buildNodeupTags error: %v", err)
 	}
@@ -218,7 +218,7 @@ func TestBuildTags_UpdatePolicy_Nil(t *testing.T) {
 		t.Fatalf("buildCloudupTags error: %v", err)
 	}
 
-	nodeUpTags, err := buildNodeupTags(c, tags)
+	nodeUpTags, err := buildNodeupTags(api.InstanceGroupRoleNode, c, tags)
 	if err != nil {
 		t.Fatalf("buildNodeupTags error: %v", err)
 	}
@@ -236,7 +236,7 @@ func TestBuildTags_UpdatePolicy_None(t *testing.T) {
 		t.Fatalf("buildTags error: %v", err)
 	}
 
-	nodeUpTags, err := buildNodeupTags(c, tags)
+	nodeUpTags, err := buildNodeupTags(api.InstanceGroupRoleNode, c, tags)
 	if err != nil {
 		t.Fatalf("buildNodeupTags error: %v", err)
 	}
@@ -262,7 +262,7 @@ func TestBuildTags_CloudProvider_AWS_Cilium(t *testing.T) {
 		t.Fatal("tag _aws not found")
 	}
 
-	nodeUpTags, err := buildNodeupTags(c, tags)
+	nodeUpTags, err := buildNodeupTags(api.InstanceGroupRoleNode, c, tags)
 	if err != nil {
 		t.Fatalf("buildNodeupTags error: %v", err)
 	}