Merge pull request #284 from zzxwill/init-syntax-issues

Check whether there is error in Terraform init stage

api/types/state.go

@@ -33,6 +33,15 @@ const (
 	ConfigurationReloading               ConfigurationState = "ConfigurationReloading"
 	GeneratingOutputs                    ConfigurationState = "GeneratingTerraformOutputs"
 	InvalidRegion                        ConfigurationState = "InvalidRegion"
+	TerraformInitError                   ConfigurationState = "TerraformInitError"
+)
+
+// Stage is the Terraform stage
+type Stage string
+
+const (
+	TerraformInit  Stage = "TerraformInit"
+	TerraformApply Stage = "TerraformApply"
 )
 
 const (

chart/crds/terraform.core.oam.dev_configurations.yaml

@@ -187,10 +187,6 @@ spec:
           spec:
             description: ConfigurationSpec defines the desired state of Configuration
             properties:
-              JSON:
-                description: 'JSON is the Terraform JSON syntax configuration. Deprecated:
-                  after v0.3.1, use HCL instead.'
-                type: string
               backend:
                 description: Backend stores the state in a Kubernetes secret with
                   locking done using a Lease resource. TODO(zzxwill) If a backend

controllers/configuration/configuration.go

@@ -120,7 +120,7 @@ func IsDeletable(ctx context.Context, k8sClient client.Client, configuration *v1
 	}
 	// allow Configuration to delete when the Provider doesn't exist or is not ready, which means external cloud resources are
 	// not provisioned at all
-	if providerObj == nil || providerObj.Status.State == types.ProviderIsNotReady {
+	if providerObj == nil || providerObj.Status.State == types.ProviderIsNotReady || configuration.Status.Apply.State == types.TerraformInitError {
 		return true, nil
 	}
 

controllers/configuration_controller.go

@@ -62,7 +62,8 @@ const (
 	// BackendVolumeMountPath is the volume mount path for Terraform backend
 	BackendVolumeMountPath = "/opt/tf-backend"
 	// terraformContainerName is the name of the container that executes the terraform in the pod
-	terraformContainerName = "terraform-executor"
+	terraformContainerName     = "terraform-executor"
+	terraformInitContainerName = "terraform-init"
 )
 
 const (
@@ -145,7 +146,7 @@ func (r *ConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		// terraform destroy
 		klog.InfoS("performing Configuration Destroy", "Namespace", req.Namespace, "Name", req.Name, "JobName", meta.DestroyJobName)
 
-		_, err := terraform.GetTerraformStatus(ctx, meta.Namespace, meta.DestroyJobName, terraformContainerName)
+		_, err := terraform.GetTerraformStatus(ctx, meta.Namespace, meta.DestroyJobName, terraformContainerName, terraformInitContainerName)
 		if err != nil {
 			klog.ErrorS(err, "Terraform destroy failed")
 			if updateErr := meta.updateDestroyStatus(ctx, r.Client, types.ConfigurationDestroyFailed, err.Error()); updateErr != nil {
@@ -181,7 +182,7 @@ func (r *ConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		}
 		return ctrl.Result{RequeueAfter: 3 * time.Second}, errors.Wrap(err, "failed to create/update cloud resource")
 	}
-	state, err := terraform.GetTerraformStatus(ctx, meta.Namespace, meta.ApplyJobName, terraformContainerName)
+	state, err := terraform.GetTerraformStatus(ctx, meta.Namespace, meta.ApplyJobName, terraformContainerName, terraformInitContainerName)
 	if err != nil {
 		klog.ErrorS(err, "Terraform apply failed")
 		if updateErr := meta.updateApplyStatus(ctx, r.Client, state, err.Error()); updateErr != nil {
@@ -577,11 +578,12 @@ func (meta *TFConfigurationMeta) updateTerraformJobIfNeeded(ctx context.Context,
 
 func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExecutionType) *batchv1.Job {
 	var (
-		initContainer  v1.Container
-		initContainers []v1.Container
-		parallelism    int32 = 1
-		completions    int32 = 1
-		backoffLimit   int32 = math.MaxInt32
+		initContainer           v1.Container
+		tfPreApplyInitContainer v1.Container
+		initContainers          []v1.Container
+		parallelism             int32 = 1
+		completions             int32 = 1
+		backoffLimit            int32 = math.MaxInt32
 	)
 
 	executorVolumes := meta.assembleExecutorVolumes()
@@ -600,6 +602,7 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe
 		},
 	}
 
+	// prepare local Terraform .tf files
 	initContainer = v1.Container{
 		Name:            "prepare-input-terraform-configurations",
 		Image:           meta.BusyboxImage,
@@ -611,6 +614,7 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe
 		},
 		VolumeMounts: initContainerVolumeMounts,
 	}
+
 	initContainers = append(initContainers, initContainer)
 
 	hclPath := filepath.Join(BackendVolumeMountPath, meta.RemoteGitPath)
@@ -631,6 +635,20 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe
 			})
 	}
 
+	// run `terraform init`
+	tfPreApplyInitContainer = v1.Container{
+		Name:            terraformInitContainerName,
+		Image:           meta.TerraformImage,
+		ImagePullPolicy: v1.PullIfNotPresent,
+		Command: []string{
+			"sh",
+			"-c",
+			"terraform init",
+		},
+		VolumeMounts: initContainerVolumeMounts,
+	}
+	initContainers = append(initContainers, tfPreApplyInitContainer)
+
 	return &batchv1.Job{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Job",
@@ -658,15 +676,15 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe
 					// state file directory in advance
 					InitContainers: initContainers,
 					// Container terraform-executor will first copy predefined terraform.d to working directory, and
-					// then run terraform init/apply.
+					// then run terraform apply/destroy.
 					Containers: []v1.Container{{
 						Name:            terraformContainerName,
 						Image:           meta.TerraformImage,
 						ImagePullPolicy: v1.PullIfNotPresent,
 						Command: []string{
 							"bash",
 							"-c",
-							fmt.Sprintf("terraform init && terraform %s -lock=false -auto-approve", executionType),
+							fmt.Sprintf("terraform %s -lock=false -auto-approve", executionType),
 						},
 						VolumeMounts: []v1.VolumeMount{
 							{

controllers/terraform/logging.go

@@ -10,25 +10,47 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
+
+	"github.com/oam-dev/terraform-controller/api/types"
 )
 
-func getPodLog(ctx context.Context, client kubernetes.Interface, namespace, jobName, containerName string) (string, error) {
+func getPods(ctx context.Context, client kubernetes.Interface, namespace, jobName string) (*v1.PodList, error) {
 	label := fmt.Sprintf("job-name=%s", jobName)
 	pods, err := client.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{LabelSelector: label})
-	if err != nil || pods == nil || len(pods.Items) == 0 {
+	if err != nil {
 		klog.InfoS("pods are not found", "Label", label, "Error", err)
-		return "", nil
+		return nil, err
+	}
+	return pods, nil
+}
+
+func getPodLog(ctx context.Context, client kubernetes.Interface, namespace, jobName, containerName, initContainerName string) (types.Stage, string, error) {
+	var (
+		targetContainer = containerName
+		stage           = types.TerraformApply
+	)
+	pods, err := getPods(ctx, client, namespace, jobName)
+	if err != nil || pods == nil || len(pods.Items) == 0 {
+		klog.V(4).InfoS("pods are not found", "PodName", jobName, "Namepspace", namespace, "Error", err)
+		return stage, "", nil
 	}
 	pod := pods.Items[0]
 
+	// Here are two cases for Pending phase: 1) init container `terraform init` is not finished yet, 2) pod is not ready yet.
 	if pod.Status.Phase == v1.PodPending {
-		return "", nil
+		for _, c := range pod.Status.InitContainerStatuses {
+			if c.Name == initContainerName && !c.Ready {
+				targetContainer = initContainerName
+				stage = types.TerraformInit
+				break
+			}
+		}
 	}
 
-	req := client.CoreV1().Pods(namespace).GetLogs(pod.Name, &v1.PodLogOptions{Container: containerName})
+	req := client.CoreV1().Pods(namespace).GetLogs(pod.Name, &v1.PodLogOptions{Container: targetContainer})
 	logs, err := req.Stream(ctx)
 	if err != nil {
-		return "", err
+		return stage, "", err
 	}
 	defer func(logs io.ReadCloser) {
 		err := logs.Close()
@@ -37,7 +59,9 @@ func getPodLog(ctx context.Context, client kubernetes.Interface, namespace, jobN
 		}
 	}(logs)
 
-	return flushStream(logs, pod.Name)
+	log, err := flushStream(logs, pod.Name)
+
+	return stage, log, err
 }
 
 func flushStream(rc io.ReadCloser, podName string) (string, error) {

controllers/terraform/logging_test.go

@@ -19,17 +19,21 @@ import (
 	"k8s.io/client-go/kubernetes/typed/core/v1/fake"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/util/flowcontrol"
+
+	"github.com/oam-dev/terraform-controller/api/types"
 )
 
 func TestGetPodLog(t *testing.T) {
 	ctx := context.Background()
 	type args struct {
-		client        kubernetes.Interface
-		namespace     string
-		name          string
-		containerName string
+		client            kubernetes.Interface
+		namespace         string
+		name              string
+		containerName     string
+		initContainerName string
 	}
 	type want struct {
+		state  types.Stage
 		log    string
 		errMsg string
 	}
@@ -78,10 +82,11 @@ func TestGetPodLog(t *testing.T) {
 		{
 			name: "Pod is available, but no logs",
 			args: args{
-				client:        k8sClientSet,
-				namespace:     "default",
-				name:          "j1",
-				containerName: "terraform-executor",
+				client:            k8sClientSet,
+				namespace:         "default",
+				name:              "j1",
+				containerName:     "terraform-executor",
+				initContainerName: "terraform-init",
 			},
 			want: want{
 				errMsg: "can not be accept",
@@ -90,12 +95,12 @@ func TestGetPodLog(t *testing.T) {
 	}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			got, err := getPodLog(ctx, tc.args.client, tc.args.namespace, tc.args.name, tc.args.containerName)
-			if tc.want.errMsg != "" {
+			state, got, err := getPodLog(ctx, tc.args.client, tc.args.namespace, tc.args.name, tc.args.containerName, tc.args.initContainerName)
+			if tc.want.errMsg != "" || err != nil {
 				assert.EqualError(t, err, tc.want.errMsg)
 			} else {
-				assert.NoError(t, err)
 				assert.Equal(t, tc.want.log, got)
+				assert.Equal(t, tc.want.state, state)
 			}
 		})
 	}

controllers/terraform/status.go

@@ -12,37 +12,44 @@ import (
 )
 
 // GetTerraformStatus will get Terraform execution status
-func GetTerraformStatus(ctx context.Context, namespace, jobName, containerName string) (types.ConfigurationState, error) {
-	klog.InfoS("checking Terraform execution status", "Namespace", namespace, "Job", jobName)
+func GetTerraformStatus(ctx context.Context, namespace, jobName, containerName, initContainerName string) (types.ConfigurationState, error) {
+	klog.InfoS("checking Terraform init and execution status", "Namespace", namespace, "Job", jobName)
 	clientSet, err := client.Init()
 	if err != nil {
 		klog.ErrorS(err, "failed to init clientSet")
 		return types.ConfigurationProvisioningAndChecking, err
 	}
 
-	logs, err := getPodLog(ctx, clientSet, namespace, jobName, containerName)
+	// check the stage of the pod
+
+	stage, logs, err := getPodLog(ctx, clientSet, namespace, jobName, containerName, initContainerName)
 	if err != nil {
 		klog.ErrorS(err, "failed to get pod logs")
 		return types.ConfigurationProvisioningAndChecking, err
 	}
 
-	success, state, errMsg := analyzeTerraformLog(logs)
+	success, state, errMsg := analyzeTerraformLog(logs, stage)
 	if success {
 		return state, nil
 	}
 
 	return state, errors.New(errMsg)
 }
 
-func analyzeTerraformLog(logs string) (bool, types.ConfigurationState, string) {
+func analyzeTerraformLog(logs string, stage types.Stage) (bool, types.ConfigurationState, string) {
 	lines := strings.Split(logs, "\n")
 	for i, line := range lines {
 		if strings.Contains(line, "31mError:") {
 			errMsg := strings.Join(lines[i:], "\n")
 			if strings.Contains(errMsg, "Invalid Alibaba Cloud region") {
 				return false, types.InvalidRegion, errMsg
 			}
-			return false, types.ConfigurationApplyFailed, errMsg
+			switch stage {
+			case types.TerraformInit:
+				return false, types.TerraformInitError, errMsg
+			case types.TerraformApply:
+				return false, types.ConfigurationApplyFailed, errMsg
+			}
 		}
 	}
 	return true, types.ConfigurationProvisioningAndChecking, ""

controllers/terraform/status_test.go

@@ -49,7 +49,7 @@ func TestGetTerraformStatus(t *testing.T) {
 	}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			state, err := GetTerraformStatus(ctx, tc.args.namespace, tc.args.name, tc.args.containerName)
+			state, err := GetTerraformStatus(ctx, tc.args.namespace, tc.args.name, tc.args.containerName, "")
 			if tc.want.errMsg != "" {
 				assert.EqualError(t, err, tc.want.errMsg)
 			} else {
@@ -92,7 +92,7 @@ func TestGetTerraformStatus2(t *testing.T) {
 	}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			state, err := GetTerraformStatus(ctx, tc.args.namespace, tc.args.name, tc.args.containerName)
+			state, err := GetTerraformStatus(ctx, tc.args.namespace, tc.args.name, tc.args.containerName, "")
 			if tc.want.errMsg != "" {
 				assert.Contains(t, err.Error(), tc.want.errMsg)
 			} else {
@@ -143,7 +143,7 @@ func TestAnalyzeTerraformLog(t *testing.T) {
 	}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			success, state, errMsg := analyzeTerraformLog(tc.args.logs)
+			success, state, errMsg := analyzeTerraformLog(tc.args.logs, types.TerraformApply)
 			if tc.want.errMsg != "" {
 				assert.Contains(t, errMsg, tc.want.errMsg)
 			} else {

controllers/util/decompress_test.go

@@ -9,7 +9,8 @@ import (
 
 func TestDecompressTerraformStateSecret(t *testing.T) {
 	type args struct {
-		data string
+		data       string
+		needDecode bool
 	}
 	type want struct {
 		raw    string
@@ -23,7 +24,8 @@ func TestDecompressTerraformStateSecret(t *testing.T) {
 		{
 			name: "decompress terraform state secret",
 			args: args{
-				data: "H4sIAAAAAAAA/0SMwa7CIBBF9/0KMutH80ArDb9ijKHDYEhqMQO4afrvBly4POfc3H0QAt7EOaYNrDj/NS7E7ELi5/1XQI3/o4beM3F0K1ihO65xI/egNsLThLPRWi6agkR/CVIppaSZJrfgbBx6//1ItbxqyWDFfnTBlFNlpKaut+EYPgEAAP//xUXpvZsAAAA=",
+				data:       "H4sIAAAAAAAA/0SMwa7CIBBF9/0KMutH80ArDb9ijKHDYEhqMQO4afrvBly4POfc3H0QAt7EOaYNrDj/NS7E7ELi5/1XQI3/o4beM3F0K1ihO65xI/egNsLThLPRWi6agkR/CVIppaSZJrfgbBx6//1ItbxqyWDFfnTBlFNlpKaut+EYPgEAAP//xUXpvZsAAAA=",
+				needDecode: true,
 			},
 			want: want{
 				raw: `{
@@ -37,14 +39,26 @@ func TestDecompressTerraformStateSecret(t *testing.T) {
 `,
 			},
 		},
+		{
+			name: "bad data",
+			args: args{
+				data: "abc",
+			},
+			want: want{
+				errMsg: "EOF",
+			},
+		},
 	}
 
 	for _, tt := range testcases {
 		t.Run(tt.name, func(t *testing.T) {
-			state, err := base64.StdEncoding.DecodeString(tt.args.data)
-			assert.NoError(t, err)
-			got, err := DecompressTerraformStateSecret(string(state))
-			if tt.want.errMsg != "" {
+			if tt.args.needDecode {
+				state, err := base64.StdEncoding.DecodeString(tt.args.data)
+				assert.NoError(t, err)
+				tt.args.data = string(state)
+			}
+			got, err := DecompressTerraformStateSecret(tt.args.data)
+			if tt.want.errMsg != "" || err != nil {
 				assert.Contains(t, err.Error(), tt.want.errMsg)
 			} else {
 				assert.Equal(t, tt.want.raw, string(got))