RunCmd method changed

agent/cloudinit/cloudinit.go

@@ -4,6 +4,7 @@
 package cloudinit
 
 import (
+	"context"
 	"encoding/base64"
 	"fmt"
 	"path/filepath"
@@ -71,7 +72,7 @@ func (se ScriptExecutor) Execute(bootstrapScript string) error {
 	}
 
 	for _, cmd := range cloudInitData.CommandsToExecute {
-		err := se.RunCmdExecutor.RunCmd(cmd)
+		err := se.RunCmdExecutor.RunCmd(context.TODO(), cmd)
 		if err != nil {
 			return errors.Wrap(err, fmt.Sprintf("Error running the command %s", cmd))
 		}

agent/cloudinit/cloudinit_test.go

@@ -135,7 +135,7 @@ runCmd:
 			Expect(err).NotTo(HaveOccurred())
 
 			Expect(fakeCmdExecutor.RunCmdCallCount()).To(Equal(1))
-			cmd := fakeCmdExecutor.RunCmdArgsForCall(0)
+			_, cmd := fakeCmdExecutor.RunCmdArgsForCall(0)
 			Expect(cmd).To(Equal("echo 'some run command'"))
 		})
 

agent/cloudinit/cmd_runner.go

@@ -4,13 +4,14 @@
 package cloudinit
 
 import (
+	"context"
 	"os"
 	"os/exec"
 )
 
 //counterfeiter:generate . ICmdRunner
 type ICmdRunner interface {
-	RunCmd(string) error
+	RunCmd(context.Context, string) error
 }
 
 // CmdRunner default implementer of ICmdRunner
@@ -19,8 +20,12 @@ type CmdRunner struct {
 }
 
 // RunCmd executes the command string
-func (r CmdRunner) RunCmd(cmd string) error {
-	command := exec.Command("/bin/sh", "-c", cmd)
+func (r CmdRunner) RunCmd(ctx context.Context, cmd string) error {
+	command := exec.CommandContext(ctx, "/bin/bash", "-c", cmd)
 	command.Stderr = os.Stderr
-	return command.Run()
+	command.Stdout = os.Stdout
+	if err := command.Run(); err != nil {
+		return err
+	}
+	return nil
 }

agent/reconciler/host_reconciler.go

@@ -7,7 +7,6 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"os/exec"
 
 	"github.com/pkg/errors"
 	"github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/cloudinit"
@@ -188,7 +187,7 @@ func (r *HostReconciler) executeInstallerController(ctx context.Context, byoHost
 		return err
 	}
 	logger.Info("executing install script")
-	err = r.executeScript(ctx, installScript)
+	err = r.CmdRunner.RunCmd(ctx, installScript)
 	if err != nil {
 		logger.Error(err, "error executing installation script")
 		r.Recorder.Event(byoHost, corev1.EventTypeWarning, "InstallScriptExecutionFailed", "install script execution failed")
@@ -213,16 +212,6 @@ func (r *HostReconciler) getBootstrapScript(ctx context.Context, dataSecretName,
 	return bootstrapSecret, nil
 }
 
-func (r *HostReconciler) executeScript(ctx context.Context, script string) error {
-	cmd := exec.CommandContext(ctx, "/bin/bash", "-c", script)
-	cmd.Stderr = os.Stderr
-	cmd.Stdout = os.Stdout
-	if err := cmd.Run(); err != nil {
-		return err
-	}
-	return nil
-}
-
 func (r *HostReconciler) parseScript(ctx context.Context, script string) (string, error) {
 	data, err := cloudinit.TemplateParser{
 		Template: map[string]string{
@@ -290,7 +279,7 @@ func (r *HostReconciler) hostCleanUp(ctx context.Context, byoHost *infrastructur
 				logger.Error(err, "error parsing Uninstallation script")
 				return err
 			}
-			err = r.executeScript(ctx, uninstallScript)
+			err = r.CmdRunner.RunCmd(ctx, uninstallScript)
 			if err != nil {
 				logger.Error(err, "error execting Uninstallation script")
 				r.Recorder.Event(byoHost, corev1.EventTypeWarning, "UninstallScriptExecutionFailed", "uninstall script execution failed")
@@ -328,7 +317,7 @@ func (r *HostReconciler) resetNode(ctx context.Context, byoHost *infrastructurev
 	logger := ctrl.LoggerFrom(ctx)
 	logger.Info("Running kubeadm reset")
 
-	err := r.CmdRunner.RunCmd(KubeadmResetCommand)
+	err := r.CmdRunner.RunCmd(ctx, KubeadmResetCommand)
 	if err != nil {
 		r.Recorder.Event(byoHost, corev1.EventTypeWarning, "ResetK8sNodeFailed", "k8s Node Reset failed")
 		return errors.Wrapf(err, "failed to exec kubeadm reset")

agent/reconciler/reconciler_test.go

@@ -380,6 +380,7 @@ runCmd:
 						})
 
 						It("should return error if install script execution failed", func() {
+							fakeCommandRunner.RunCmdReturns(errors.New("failed to execute install script"))
 							invalidInstallationSecret := builder.Secret(ns, "invalid-test-secret").
 								WithKeyData("install", "test").
 								Build()
@@ -444,38 +445,6 @@ runCmd:
 							}))
 						})
 
-						It("should set K8sNodeBootstrapSucceeded to false with Reason CloudInitExecutionFailedReason if the bootstrap execution fails", func() {
-							fakeCommandRunner.RunCmdReturns(errors.New("I failed"))
-
-							result, reconcilerErr := hostReconciler.Reconcile(ctx, controllerruntime.Request{
-								NamespacedName: byoHostLookupKey,
-							})
-
-							Expect(result).To(Equal(controllerruntime.Result{}))
-							Expect(reconcilerErr).To(HaveOccurred())
-
-							updatedByoHost := &infrastructurev1beta1.ByoHost{}
-							err := k8sClient.Get(ctx, byoHostLookupKey, updatedByoHost)
-							Expect(err).ToNot(HaveOccurred())
-
-							k8sNodeBootstrapSucceeded := conditions.Get(updatedByoHost, infrastructurev1beta1.K8sNodeBootstrapSucceeded)
-							Expect(*k8sNodeBootstrapSucceeded).To(conditions.MatchCondition(clusterv1.Condition{
-								Type:     infrastructurev1beta1.K8sNodeBootstrapSucceeded,
-								Status:   corev1.ConditionFalse,
-								Reason:   infrastructurev1beta1.CloudInitExecutionFailedReason,
-								Severity: clusterv1.ConditionSeverityError,
-							}))
-
-							// assert events
-							events := eventutils.CollectEvents(recorder.Events)
-							Expect(events).Should(ConsistOf([]string{
-								"Normal InstallScriptExecutionSucceeded install script executed",
-								"Warning BootstrapK8sNodeFailed k8s Node Bootstrap failed",
-								// TODO: improve test to remove this event
-								"Warning ResetK8sNodeFailed k8s Node Reset failed",
-							}))
-						})
-
 						It("should set K8sNodeBootstrapSucceeded to True if the boostrap execution succeeds", func() {
 							hostReconciler.K8sInstaller = fakeInstaller
 							result, reconcilerErr := hostReconciler.Reconcile(ctx, controllerruntime.Request{
@@ -484,7 +453,7 @@ runCmd:
 							Expect(result).To(Equal(controllerruntime.Result{}))
 							Expect(reconcilerErr).ToNot(HaveOccurred())
 
-							Expect(fakeCommandRunner.RunCmdCallCount()).To(Equal(1))
+							Expect(fakeCommandRunner.RunCmdCallCount()).To(Equal(2))
 							Expect(fakeFileWriter.WriteToFileCallCount()).To(Equal(1))
 
 							updatedByoHost := &infrastructurev1beta1.ByoHost{}
@@ -577,7 +546,8 @@ runCmd:
 
 				// assert kubeadm reset is called
 				Expect(fakeCommandRunner.RunCmdCallCount()).To(Equal(1))
-				Expect(fakeCommandRunner.RunCmdArgsForCall(0)).To(Equal(reconciler.KubeadmResetCommand))
+				_, resetCommand := fakeCommandRunner.RunCmdArgsForCall(0)
+				Expect(resetCommand).To(Equal(reconciler.KubeadmResetCommand))
 				updatedByoHost := &infrastructurev1beta1.ByoHost{}
 				err := k8sClient.Get(ctx, byoHostLookupKey, updatedByoHost)
 				Expect(err).ToNot(HaveOccurred())
@@ -622,6 +592,7 @@ runCmd:
 				})
 
 				It("should return error if uninstall script execution failed ", func() {
+					fakeCommandRunner.RunCmdReturnsOnCall(1, errors.New("failed to execute uninstall script"))
 					uninstallScript = `testcommand`
 					byoHost.Spec.UninstallationScript = &uninstallScript
 					Expect(patchHelper.Patch(ctx, byoHost, patch.WithStatusObservedGeneration{})).NotTo(HaveOccurred())