Helm install updates, fix PR build (#39)

- Adds mandatory account_id parameter to helm charts
- Fixes to documentation
- Enable PR build. We can fix the failing tests in a different PR. Lets have the tests running for all PRs

### Testing
Manual

Author: surajkota

README.md

@@ -186,7 +186,7 @@ kubectl apply -f $CHART_EXPORT_PATH/ack-$SERVICE-controller/crds
 Create a namespace and install the helm chart
 ```sh
 export ACK_K8S_NAMESPACE=ack-system
-helm install -n $ACK_K8S_NAMESPACE --create-namespace $ACK_K8S_NAMESPACE --skip-crds ack-$SERVICE-controller \
+helm install -n $ACK_K8S_NAMESPACE --create-namespace --skip-crds ack-$SERVICE-controller \
  $CHART_EXPORT_PATH/ack-$SERVICE-controller 
 ```
 

samples/README.md

@@ -9,7 +9,7 @@ Follow the instructions on our [installation page](/README.md#getting-started) t
 Run the following commands to create a SageMaker execution IAM role which is used by SageMaker service to access AWS resources. 
 
 ```
-export SAGEMAKER_EXECUTION_ROLE_NAME=ack-sagemaker-execution-role-$CLUSTER_NAME
+export SAGEMAKER_EXECUTION_ROLE_NAME=ack-sagemaker-execution-role
 
 TRUST="{ \"Version\": \"2012-10-17\", \"Statement\": [ { \"Effect\": \"Allow\", \"Principal\": { \"Service\": \"sagemaker.amazonaws.com\" }, \"Action\": \"sts:AssumeRole\" } ] }"
 aws iam create-role --role-name ${SAGEMAKER_EXECUTION_ROLE_NAME} --assume-role-policy-document "$TRUST"

test/canary/scripts/install_controller_helm.sh

@@ -7,10 +7,12 @@ function install_helm_chart() {
     local oidc_role_arn="$2"
     local region="$3"
     local namespace="$4"
+    local account_id=$(aws sts get-caller-identity --output text --query "Account")
 
     yq w -i helm/values.yaml "serviceAccount.annotations" ""
     yq w -i helm/values.yaml 'serviceAccount.annotations."eks.amazonaws.com/role-arn"' "$oidc_role_arn"
     yq w -i helm/values.yaml "aws.region" $region
+    yq w -i helm/values.yaml "aws.account_id" $account_id
 
     kubectl create namespace $namespace
     kubectl apply -f helm/crds

test/e2e/service_bootstrap.py

@@ -90,23 +90,29 @@ def create_data_bucket() -> str:
     source_bucket = s3_resource.Bucket(SAGEMAKER_SOURCE_DATA_BUCKET)
     destination_bucket = s3_resource.Bucket(bucket_name)
     temp_dir = "/tmp/ack_s3_data"
-    # duplicate_bucket_contents(source_bucket, destination_bucket)
-    # workaround to copy if buckets are across regions
-    # TODO: check if there is a better way and merge to test-infra
-    subprocess.call(["mkdir", f"{temp_dir}"])
-    subprocess.call(
-        [
-            "aws",
-            "s3",
-            "sync",
-            f"s3://{SAGEMAKER_SOURCE_DATA_BUCKET}",
-            f"./{temp_dir}/",
-            "--quiet",
-        ]
-    )
-    subprocess.call(
-        ["aws", "s3", "sync", f"./{temp_dir}/", f"s3://{bucket_name}", "--quiet"]
-    )
+    # awscli is not installed in test-infra container hence use boto3 to copy in us-west-2
+    if region == "us-west-2":
+        duplicate_bucket_contents(source_bucket, destination_bucket)
+        # above method does an async copy
+        # TODO: find a way to remove random wait
+        time.sleep(180)
+    else:
+        # workaround to copy if buckets are across regions
+        # TODO: check if there is a better way and merge to test-infra
+        subprocess.call(["mkdir", f"{temp_dir}"])
+        subprocess.call(
+            [
+                "aws",
+                "s3",
+                "sync",
+                f"s3://{SAGEMAKER_SOURCE_DATA_BUCKET}",
+                f"./{temp_dir}/",
+                "--quiet",
+            ]
+        )
+        subprocess.call(
+            ["aws", "s3", "sync", f"./{temp_dir}/", f"s3://{bucket_name}", "--quiet"]
+        )
 
     logging.info(f"Synced data bucket")