Use ansible/terraform modules over go/exec

[kishen-v]

This PR simplifies implementations for executing terraform/ansible sections through modules and has been tested end-to-end with available options.

Fixes: #9

[kishen-v]

The module has no support for the -auto-approve option and is baked into the module for all terraform operations.
For e.g: tf.Apply

[kishen-v]

Since auto-approved is no longer supported, setting/unsetting will not have any effect on the execution.

[mkumatag]

interesting to see that they are always setting the auto-approve in the code - https://github.com/hashicorp/terraform-exec/blob/7ae12dd2176573ec55c9b5c84542d1b466efce54/tfexec/apply.go#L176

Maybe for later: explore whether we can influence the community to support this option, as it would be helpful for a dry‑run test to see what changes are occurring before the actual run.

[kishen-v]

Created an issue on the terraform exec repo just to get an opinion!

[mkumatag]

@kishen-v can you please create an issue and add all the information about the change you are trying to make and why?

[mkumatag]

/hold
wait till we have some conformance job running

[kishen-v]

/test pull-provider-ibmcloud-test-infra-kubernetes

[kishen-v]

/cc @Prajyot-Parab
Thanks!

[Prajyot-Parab]

/lgtm
/cc @Amulyam24

[Amulyam24]

Golang changes are okay, I might not be the right person to review the flow, will defer it to @Rajalakshmi-Girish, Thanks @kishen-v!

[mkumatag]

interesting to see that they are always setting the auto-approve in the code - https://github.com/hashicorp/terraform-exec/blob/7ae12dd2176573ec55c9b5c84542d1b466efce54/tfexec/apply.go#L176

Maybe for later: explore whether we can influence the community to support this option, as it would be helpful for a dry‑run test to see what changes are occurring before the actual run.

[mkumatag]

this mapping can be avoid if change the datatype for the extraVars to map[string]any.

Here is the diff, see if this works:

diff --git a/kubetest2-tf/deployer/deployer.go b/kubetest2-tf/deployer/deployer.go
index dd1887a..c1936b3 100644
--- a/kubetest2-tf/deployer/deployer.go
+++ b/kubetest2-tf/deployer/deployer.go
@@ -76,15 +76,15 @@ type deployer struct {
 	tmpDir        string
 	machineIPs    []string
 
-	RepoRoot              string            `desc:"The path to the root of the local kubernetes repo. Necessary to call certain scripts. Defaults to the current directory. If operating in legacy mode, this should be set to the local kubernetes/kubernetes repo."`
-	IgnoreClusterDir      bool              `desc:"Ignore the cluster folder if exists"`
-	AutoApprove           bool              `desc:"Auto-approve the deployment of infrastructure through terraform" flag:",deprecated"`
-	RetryOnTfFailure      int               `desc:"Retry on Terraform Apply Failure"`
-	BreakKubetestOnUpfail bool              `desc:"Breaks kubetest2 when up fails"`
-	Playbook              string            `desc:"Name of ansible playbook to be run"`
-	ExtraVars             map[string]string `desc:"Passes extra-vars to ansible playbook, enter a string of key=value pairs"`
-	SetKubeconfig         bool              `desc:"Flag to set kubeconfig"`
-	TargetProvider        string            `desc:"provider value to be used(powervs, vpc)"`
+	RepoRoot              string         `desc:"The path to the root of the local kubernetes repo. Necessary to call certain scripts. Defaults to the current directory. If operating in legacy mode, this should be set to the local kubernetes/kubernetes repo."`
+	IgnoreClusterDir      bool           `desc:"Ignore the cluster folder if exists"`
+	AutoApprove           bool           `desc:"Auto-approve the deployment of infrastructure through terraform" flag:",deprecated"`
+	RetryOnTfFailure      int            `desc:"Retry on Terraform Apply Failure"`
+	BreakKubetestOnUpfail bool           `desc:"Breaks kubetest2 when up fails"`
+	Playbook              string         `desc:"Name of ansible playbook to be run"`
+	ExtraVars             map[string]any `desc:"Passes extra-vars to ansible playbook, enter a string of key=value pairs"`
+	SetKubeconfig         bool           `desc:"Flag to set kubeconfig"`
+	TargetProvider        string         `desc:"provider value to be used(powervs, vpc)"`
 }
 
 func (d *deployer) Version() string {
@@ -247,7 +247,7 @@ func (d *deployer) Up() error {
 	}
 	klog.Infof("Ansible params exposed under groupvars: %v", string(ansibleParams))
 	// Unmarshalling commonJSON into map to add extra-vars
-	combinedAnsibleVars := map[string]string{}
+	combinedAnsibleVars := map[string]any{}
 	if err = json.Unmarshal(ansibleParams, &combinedAnsibleVars); err != nil {
 		return fmt.Errorf("failed to unmarshal ansible group variables: %v", err)
 	}
diff --git a/kubetest2-tf/pkg/ansible/ansible.go b/kubetest2-tf/pkg/ansible/ansible.go
index 6782c37..31f7ef8 100644
--- a/kubetest2-tf/pkg/ansible/ansible.go
+++ b/kubetest2-tf/pkg/ansible/ansible.go
@@ -17,20 +17,20 @@ const (
 	ansibleDataDir = "k8s-ansible"
 )
 
-func Playbook(dir, inventory, playbook string, extraVars map[string]string) error {
+func Playbook(dir, inventory, playbook string, extraVars map[string]any) error {
 	if err := unpackAnsible(dir); err != nil {
 		return fmt.Errorf("failed to unpack the ansible code: %v", err)
 	}
-	extraVarsMap := make(map[string]interface{}, len(extraVars))
-	for key, val := range extraVars {
-		extraVarsMap[key] = val
-	}
+	// extraVarsMap := make(map[string]interface{}, len(extraVars))
+	// for key, val := range extraVars {
+	// 	extraVarsMap[key] = val
+	// }
 	klog.Infof("Running ansible playbook: %s", playbook)
 	playbookCmd := ansibleplaybook.NewAnsiblePlaybookCmd(
 		ansibleplaybook.WithPlaybooks(filepath.Join(dir, playbook)),
 		ansibleplaybook.WithPlaybookOptions(
 			&ansibleplaybook.AnsiblePlaybookOptions{
-				ExtraVars: extraVarsMap,
+				ExtraVars: extraVars,
 				Inventory: inventory,
 			}),
 	)

[mkumatag]

same can be used in the other place as well

[kishen-v]

Had tried this approach earlier, It ends up with the CLI returning an error that --extra-vars is an unknown flag:
spf13/pflag#375 - Generics aren't supported yet.😞 Given that the repo uses pflag as the underlying package to support flags..

Flag --auto-approve has been deprecated, Auto-approve the deployment of infrastructure through terraform
Error: unknown flag: --extra-vars

Usage:
  kubetest2 tf [Flags] [DeployerFlags] -- [TesterArgs]

[kishen-v]

/hold for addressing review comments and testing.

[kishen-v]

/test pull-provider-ibmcloud-test-infra-kubernetes

[kishen-v]

/test pull-provider-ibmcloud-test-infra-kubernetes

[kishen-v]

/test pull-provider-ibmcloud-test-infra-kubernetes
Squashed all changes with a bit of idiomatic handling of errs.

[mkumatag]

@kishen-v: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-provider-ibmcloud-test-infra-kubernetes 2f30e64 link true /test pull-provider-ibmcloud-test-infra-kubernetes
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

@kishen-v I see some error while tf apply retry flow, not sure if this is because of this change, ptal

[kishen-v]

Hi @mkumatag, I hadn't changed much around the Apply/Retry logic.. I did run the deployer in my local with the latest changes, it worked as expected.. However in the prow logs, I see that the LPAR deployment timed out.. Should it be worth retesting?

[kishen-v]

/test pull-provider-ibmcloud-test-infra-kubernetes

[kishen-v]

/unhold

[mkumatag]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kishen-v, mkumatag

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [mkumatag]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment