Merge pull request #3790 from KashifSaadat/bootstrapscript-hooks-bugfix

Automatic merge from submit-queue.

Bugfix kops update always detecting changes when using ExecContainerAction.

`ExecContainerAction` is referenced as a pointer in the `HookSpec`. When the bootstrapscript is run, it fingerprints the content here and stores it on-top of the existing `ExecContainerAction` within the Hook being parsed. The bootstrapscript is called for every Instance Group, and so the fingerprinted content gets passed and re-parsed.

This PR fixes the issue by creating a new `ExecContainerAction` object and assigning it to the hook being processed. Tests should now cover this case by running the `ResourceNodeUp` fn multiple times.

Fixes #3516

pkg/model/bootstrapscript.go

@@ -197,11 +197,17 @@ func (b *BootstrapScript) getRelevantHooks(allHooks []kops.HookSpec, role kops.I
 			}
 
 			if hook.ExecContainer != nil && hook.ExecContainer.Command != nil {
-				execContainerCommandFingerprint, err := b.computeFingerprint(hook.ExecContainer.Command)
+				execContainerCommandFingerprint, err := b.computeFingerprint(strings.Join(hook.ExecContainer.Command[:], " "))
 				if err != nil {
 					return nil, err
 				}
-				hook.ExecContainer.Command = []string{execContainerCommandFingerprint + " (fingerprint)"}
+
+				execContainerAction := &kops.ExecContainerAction{
+					Command:     []string{execContainerCommandFingerprint + " (fingerprint)"},
+					Environment: hook.ExecContainer.Environment,
+					Image:       hook.ExecContainer.Image,
+				}
+				hook.ExecContainer = execContainerAction
 			}
 
 			hook.Roles = nil
@@ -248,16 +254,11 @@ func (b *BootstrapScript) getRelevantFileAssets(allFileAssets []kops.FileAssetSp
 	return fileAssets, nil
 }
 
-// computeFingerprint takes an object and returns a base64 encoded fingerprint
-func (b *BootstrapScript) computeFingerprint(obj interface{}) (string, error) {
+// computeFingerprint takes a string and returns a base64 encoded fingerprint
+func (b *BootstrapScript) computeFingerprint(content string) (string, error) {
 	hasher := sha1.New()
 
-	data, err := kops.ToRawYaml(obj)
-	if err != nil {
-		return "", err
-	}
-
-	if _, err := hasher.Write(data); err != nil {
+	if _, err := hasher.Write([]byte(content)); err != nil {
 		return "", fmt.Errorf("error computing fingerprint hash: %v", err)
 	}
 

pkg/model/bootstrapscript_test.go

@@ -124,6 +124,12 @@ func TestBootstrapUserData(t *testing.T) {
 			NodeUpConfigBuilder: renderNodeUpConfig,
 		}
 
+		// Purposely running this twice to cover issue #3516
+		_, err := bs.ResourceNodeUp(group, &spec)
+		if err != nil {
+			t.Errorf("case %d failed to create nodeup resource. error: %s", i, err)
+			continue
+		}
 		res, err := bs.ResourceNodeUp(group, &spec)
 		if err != nil {
 			t.Errorf("case %d failed to create nodeup resource. error: %s", i, err)
@@ -209,7 +215,7 @@ func makeTestCluster(hookSpecRoles []kops.InstanceGroupRole, fileAssetSpecRoles
 						Command: []string{
 							"sh",
 							"-c",
-							"chroot /rootfs apt-get update && chroot /rootfs apt-get install -y ceph-common",
+							"apt-get update",
 						},
 						Image: "busybox",
 					},

pkg/model/tests/data/bootstrapscript_0.txt

@@ -177,11 +177,11 @@ __EOF_CLUSTER_SPEC
 
 cat > ig_spec.yaml << '__EOF_IG_SPEC'
 fileAssets:
-- content: tre8+iQw12cCsccJY3cQk4HQV3g= (fingerprint)
+- content: xYagtQLwBAAi3V8Wc2Jrojz28I0= (fingerprint)
   name: tokens
   path: /kube/tokens.csv
 hooks:
-- manifest: FdicqEXLciSI1yRjQxsrye3QivU= (fingerprint)
+- manifest: 8BN3anFUyDlkVF/JnaJqbwpq8ME= (fingerprint)
   name: apply-to-all.service
 kubelet:
   kubeconfigPath: /etc/kubernetes/igconfig.txt

pkg/model/tests/data/bootstrapscript_1.txt

@@ -159,13 +159,13 @@ cloudConfig:
 docker:
   logLevel: INFO
 fileAssets:
-- content: 4/KwQntXJBIluh/8K2f+zWTQdhM= (fingerprint)
+- content: E1oeAbrnQsSldrIP1BpoP2SDykM= (fingerprint)
   name: iptables-restore
   path: /var/lib/iptables/rules-save
 hooks:
 - execContainer:
     command:
-    - ChIIt0bv6sTY/wwaEWTBNaObBgM= (fingerprint)
+    - pkF7ytM3ENpYWZF36FoHJsqXP5Y= (fingerprint)
     image: busybox
 kubeAPIServer:
   image: CoreOS
@@ -186,19 +186,19 @@ __EOF_CLUSTER_SPEC
 
 cat > ig_spec.yaml << '__EOF_IG_SPEC'
 fileAssets:
-- content: 4/KwQntXJBIluh/8K2f+zWTQdhM= (fingerprint)
+- content: E1oeAbrnQsSldrIP1BpoP2SDykM= (fingerprint)
   name: iptables-restore
   path: /var/lib/iptables/rules-save
-- content: tre8+iQw12cCsccJY3cQk4HQV3g= (fingerprint)
+- content: xYagtQLwBAAi3V8Wc2Jrojz28I0= (fingerprint)
   name: tokens
   path: /kube/tokens.csv
 hooks:
 - before:
   - update-engine.service
   - kubelet.service
-  manifest: LSdpmOQebIkYoG0lRv8AUHCBIyg= (fingerprint)
+  manifest: /uSPh015xYXh8dAVqXjP/ePkbrM= (fingerprint)
   name: disable-update-engine.service
-- manifest: FdicqEXLciSI1yRjQxsrye3QivU= (fingerprint)
+- manifest: 8BN3anFUyDlkVF/JnaJqbwpq8ME= (fingerprint)
   name: apply-to-all.service
 kubelet:
   kubeconfigPath: /etc/kubernetes/igconfig.txt

pkg/model/tests/data/bootstrapscript_2.txt

@@ -159,13 +159,13 @@ cloudConfig:
 docker:
   logLevel: INFO
 fileAssets:
-- content: 4/KwQntXJBIluh/8K2f+zWTQdhM= (fingerprint)
+- content: E1oeAbrnQsSldrIP1BpoP2SDykM= (fingerprint)
   name: iptables-restore
   path: /var/lib/iptables/rules-save
 hooks:
 - execContainer:
     command:
-    - ChIIt0bv6sTY/wwaEWTBNaObBgM= (fingerprint)
+    - pkF7ytM3ENpYWZF36FoHJsqXP5Y= (fingerprint)
     image: busybox
 kubeAPIServer:
   image: CoreOS
@@ -186,19 +186,19 @@ __EOF_CLUSTER_SPEC
 
 cat > ig_spec.yaml << '__EOF_IG_SPEC'
 fileAssets:
-- content: 4/KwQntXJBIluh/8K2f+zWTQdhM= (fingerprint)
+- content: E1oeAbrnQsSldrIP1BpoP2SDykM= (fingerprint)
   name: iptables-restore
   path: /var/lib/iptables/rules-save
-- content: tre8+iQw12cCsccJY3cQk4HQV3g= (fingerprint)
+- content: xYagtQLwBAAi3V8Wc2Jrojz28I0= (fingerprint)
   name: tokens
   path: /kube/tokens.csv
 hooks:
 - before:
   - update-engine.service
   - kubelet.service
-  manifest: LSdpmOQebIkYoG0lRv8AUHCBIyg= (fingerprint)
+  manifest: /uSPh015xYXh8dAVqXjP/ePkbrM= (fingerprint)
   name: disable-update-engine.service
-- manifest: FdicqEXLciSI1yRjQxsrye3QivU= (fingerprint)
+- manifest: 8BN3anFUyDlkVF/JnaJqbwpq8ME= (fingerprint)
   name: apply-to-all.service
 kubelet:
   kubeconfigPath: /etc/kubernetes/igconfig.txt

pkg/model/tests/data/bootstrapscript_3.txt

@@ -169,11 +169,11 @@ __EOF_CLUSTER_SPEC
 
 cat > ig_spec.yaml << '__EOF_IG_SPEC'
 fileAssets:
-- content: tre8+iQw12cCsccJY3cQk4HQV3g= (fingerprint)
+- content: xYagtQLwBAAi3V8Wc2Jrojz28I0= (fingerprint)
   name: tokens
   path: /kube/tokens.csv
 hooks:
-- manifest: FdicqEXLciSI1yRjQxsrye3QivU= (fingerprint)
+- manifest: 8BN3anFUyDlkVF/JnaJqbwpq8ME= (fingerprint)
   name: apply-to-all.service
 kubelet:
   kubeconfigPath: /etc/kubernetes/igconfig.txt

pkg/model/tests/data/bootstrapscript_4.txt

@@ -159,13 +159,13 @@ cloudConfig:
 docker:
   logLevel: INFO
 fileAssets:
-- content: 4/KwQntXJBIluh/8K2f+zWTQdhM= (fingerprint)
+- content: E1oeAbrnQsSldrIP1BpoP2SDykM= (fingerprint)
   name: iptables-restore
   path: /var/lib/iptables/rules-save
 hooks:
 - execContainer:
     command:
-    - ChIIt0bv6sTY/wwaEWTBNaObBgM= (fingerprint)
+    - pkF7ytM3ENpYWZF36FoHJsqXP5Y= (fingerprint)
     image: busybox
 kubeProxy:
   cpuRequest: 30m
@@ -178,19 +178,19 @@ __EOF_CLUSTER_SPEC
 
 cat > ig_spec.yaml << '__EOF_IG_SPEC'
 fileAssets:
-- content: 4/KwQntXJBIluh/8K2f+zWTQdhM= (fingerprint)
+- content: E1oeAbrnQsSldrIP1BpoP2SDykM= (fingerprint)
   name: iptables-restore
   path: /var/lib/iptables/rules-save
-- content: tre8+iQw12cCsccJY3cQk4HQV3g= (fingerprint)
+- content: xYagtQLwBAAi3V8Wc2Jrojz28I0= (fingerprint)
   name: tokens
   path: /kube/tokens.csv
 hooks:
 - before:
   - update-engine.service
   - kubelet.service
-  manifest: LSdpmOQebIkYoG0lRv8AUHCBIyg= (fingerprint)
+  manifest: /uSPh015xYXh8dAVqXjP/ePkbrM= (fingerprint)
   name: disable-update-engine.service
-- manifest: FdicqEXLciSI1yRjQxsrye3QivU= (fingerprint)
+- manifest: 8BN3anFUyDlkVF/JnaJqbwpq8ME= (fingerprint)
   name: apply-to-all.service
 kubelet:
   kubeconfigPath: /etc/kubernetes/igconfig.txt

pkg/model/tests/data/bootstrapscript_5.txt

@@ -159,13 +159,13 @@ cloudConfig:
 docker:
   logLevel: INFO
 fileAssets:
-- content: 4/KwQntXJBIluh/8K2f+zWTQdhM= (fingerprint)
+- content: E1oeAbrnQsSldrIP1BpoP2SDykM= (fingerprint)
   name: iptables-restore
   path: /var/lib/iptables/rules-save
 hooks:
 - execContainer:
     command:
-    - ChIIt0bv6sTY/wwaEWTBNaObBgM= (fingerprint)
+    - pkF7ytM3ENpYWZF36FoHJsqXP5Y= (fingerprint)
     image: busybox
 kubeProxy:
   cpuRequest: 30m
@@ -178,19 +178,19 @@ __EOF_CLUSTER_SPEC
 
 cat > ig_spec.yaml << '__EOF_IG_SPEC'
 fileAssets:
-- content: 4/KwQntXJBIluh/8K2f+zWTQdhM= (fingerprint)
+- content: E1oeAbrnQsSldrIP1BpoP2SDykM= (fingerprint)
   name: iptables-restore
   path: /var/lib/iptables/rules-save
-- content: tre8+iQw12cCsccJY3cQk4HQV3g= (fingerprint)
+- content: xYagtQLwBAAi3V8Wc2Jrojz28I0= (fingerprint)
   name: tokens
   path: /kube/tokens.csv
 hooks:
 - before:
   - update-engine.service
   - kubelet.service
-  manifest: LSdpmOQebIkYoG0lRv8AUHCBIyg= (fingerprint)
+  manifest: /uSPh015xYXh8dAVqXjP/ePkbrM= (fingerprint)
   name: disable-update-engine.service
-- manifest: FdicqEXLciSI1yRjQxsrye3QivU= (fingerprint)
+- manifest: 8BN3anFUyDlkVF/JnaJqbwpq8ME= (fingerprint)
   name: apply-to-all.service
 kubelet:
   kubeconfigPath: /etc/kubernetes/igconfig.txt