Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grant EKSCTL_ADMIN_ROLE admin access to eksctl clusters #933

Merged
merged 3 commits into from
Jun 17, 2021
Merged

Grant EKSCTL_ADMIN_ROLE admin access to eksctl clusters #933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6fdb52a
Grant EKSCTL_ADMIN_ROLE admin access to eksctl clusters
wongma7 Jun 11, 2021
9098337
Patch Node InstanceGroup, set Node count
wongma7 Jun 14, 2021
5e9fb10
Fix check for whether test docker image has been built
wongma7 Jun 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ test-e2e-external-eks:
CLUSTER_TYPE=eksctl \
K8S_VERSION="1.20" \
HELM_VALUES_FILE="./hack/values_eksctl.yaml" \
EKSCTL_ADMIN_ROLE="Infra-prod-KopsDeleteAllLambdaServiceRoleF1578477-1ELDFIB4KCMXV" \
AWS_REGION=us-west-2 \
AWS_AVAILABILITY_ZONES=us-west-2a,us-west-2b \
TEST_PATH=./tests/e2e-kubernetes/... \
Expand Down
2 changes: 1 addition & 1 deletion hack/e2e/ecr.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ function ecr_build_and_push() {
IMAGE_NAME=${3}
IMAGE_TAG=${4}
set +e
if docker images | grep "${IMAGE_NAME}" | grep "${IMAGE_TAG}"; then
if docker images --format "{{.Repository}}:{{.Tag}}" | grep "${IMAGE_NAME}:${IMAGE_TAG}"; then
set -e
loudecho "Assuming ${IMAGE_NAME}:${IMAGE_TAG} has been built and pushed"
else
Expand Down
9 changes: 9 additions & 0 deletions hack/e2e/eksctl.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ function eksctl_create_cluster() {
CLUSTER_FILE=${7}
KUBECONFIG=${8}
EKSCTL_PATCH_FILE=${9}
EKSCTL_ADMIN_ROLE=${10}

generate_ssh_key "${SSH_KEY_PATH}"

Expand Down Expand Up @@ -55,6 +56,14 @@ function eksctl_create_cluster() {

loudecho "Getting cluster ${CLUSTER_NAME}"
${BIN} get cluster "${CLUSTER_NAME}"

if [ -n "$EKSCTL_ADMIN_ROLE" ]; then
AWS_ACCOUNT_ID=$(aws sts get-caller-identity --query Account --output text)
ADMIN_ARN="arn:aws:iam::${AWS_ACCOUNT_ID}:role/${EKSCTL_ADMIN_ROLE}"
loudecho "Granting ${ADMIN_ARN} admin access to the cluster"
${BIN} create iamidentitymapping --cluster "${CLUSTER_NAME}" --arn "${ADMIN_ARN}" --group system:masters --username admin
fi

return $?
}

Expand Down
80 changes: 56 additions & 24 deletions hack/e2e/kops.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ function kops_create_cluster() {
CLUSTER_NAME=${2}
BIN=${3}
ZONES=${4}
INSTANCE_TYPE=${5}
K8S_VERSION=${6}
CLUSTER_FILE=${7}
KUBECONFIG=${8}
KOPS_PATCH_FILE=${9}
KOPS_STATE_FILE=${10}
NODE_COUNT=${5}
INSTANCE_TYPE=${6}
K8S_VERSION=${7}
CLUSTER_FILE=${8}
KUBECONFIG=${9}
KOPS_PATCH_FILE=${10}
KOPS_PATCH_NODE_FILE=${11}
KOPS_STATE_FILE=${12}

generate_ssh_key "${SSH_KEY_PATH}"

Expand All @@ -39,15 +41,18 @@ function kops_create_cluster() {
${BIN} create cluster --state "${KOPS_STATE_FILE}" \
--ssh-public-key="${SSH_KEY_PATH}".pub \
--zones "${ZONES}" \
--node-count=3 \
--node-count="${NODE_COUNT}" \
--node-size="${INSTANCE_TYPE}" \
--kubernetes-version="${K8S_VERSION}" \
--dry-run \
-o json \
-o yaml \
"${CLUSTER_NAME}" > "${CLUSTER_FILE}"

if test -f "$KOPS_PATCH_FILE"; then
kops_patch_cluster_file "$CLUSTER_FILE" "$KOPS_PATCH_FILE"
kops_patch_cluster_file "$CLUSTER_FILE" "$KOPS_PATCH_FILE" "Cluster" ""
fi
if test -f "$KOPS_PATCH_NODE_FILE"; then
kops_patch_cluster_file "$CLUSTER_FILE" "$KOPS_PATCH_NODE_FILE" "InstanceGroup" "Node"
fi

loudecho "Creating cluster $CLUSTER_NAME with $CLUSTER_FILE"
Expand Down Expand Up @@ -88,36 +93,63 @@ function kops_delete_cluster() {
${BIN} delete cluster --name "${CLUSTER_NAME}" --state "${KOPS_STATE_FILE}" --yes
}

# TODO switch this to python or work exclusively with yaml, all this
# hacking with jq stinks!
# TODO switch this to python, work exclusively with yaml, use kops toolbox
# template/kops set?, all this hacking with jq stinks!
function kops_patch_cluster_file() {
CLUSTER_FILE=${1} # input must be json
CLUSTER_FILE=${1} # input must be yaml
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a function I'd like to see done in something other than bash. Good candidate for moving reusable parts of this to the test-infra repository? (Not PR blocking, but something we can consider for the future).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, see the comment.

I know I have gone back and forth on this a lot, I decided we should use Go. You can see here https://github.com/kubernetes-sigs/aws-ebs-csi-driver/pull/927/files#diff-1174eb3051f9c1a9e1f65c760347ef47446d5b93c712925b446b29ff591184e8R36 I'm trying it out and it's fine.

KOPS_PATCH_FILE=${2} # input must be yaml
KIND=${3} # must be either Cluster or InstanceGroup
ROLE=${4} # must be either Master or Node

loudecho "Patching cluster $CLUSTER_NAME with $KOPS_PATCH_FILE"

# Temporary intermediate files for patching
# Temporary intermediate files for patching, don't mutate CLUSTER_FILE until
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this create the cluster.yaml file before the test? Is this something we can use KOPS_FEATURE_FLAGS="SpecOverrideFlag" for? Also, are these changes in kops.sh necessary for this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cluster.yaml gets created by the dry run create somewhere else. if it already exists, the pathcing won't hapepn.

No these changes aren't necessary, I just wanted to piggyback the changes from fsx repo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be clear the pr desription is only relevant for first commit.

commit 2 and 3 are snuck in

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, might want to break it up or update the PR description.

# the end
CLUSTER_FILE_JSON=$CLUSTER_FILE.json
CLUSTER_FILE_0=$CLUSTER_FILE.0
CLUSTER_FILE_1=$CLUSTER_FILE.1

# Output is an array of Cluster and InstanceGroups
jq '.[] | select(.kind=="Cluster")' "$CLUSTER_FILE" > "$CLUSTER_FILE_0"
# HACK convert the multiple yaml documents to an array of json objects
yaml_to_json "$CLUSTER_FILE" "$CLUSTER_FILE_JSON"

# Find the json objects to patch
FILTER=".[] | select(.kind==\"$KIND\")"
if [ -n "$ROLE" ]; then
FILTER="$FILTER | select(.spec.role==\"$ROLE\")"
fi
jq "$FILTER" "$CLUSTER_FILE_JSON" > "$CLUSTER_FILE_0"

# Patch only the Cluster
# Patch only the json objects
kubectl patch -f "$CLUSTER_FILE_0" --local --type merge --patch "$(cat "$KOPS_PATCH_FILE")" -o json > "$CLUSTER_FILE_1"
mv "$CLUSTER_FILE_1" "$CLUSTER_FILE_0"

# Write the patched Cluster back to the array
jq '(.[] | select(.kind=="Cluster")) = $cluster[0]' "$CLUSTER_FILE" --slurpfile cluster "$CLUSTER_FILE_0" > "$CLUSTER_FILE_1"
# Delete the original json objects, add the patched
# TODO Cluster must always be first?
jq "del($FILTER)" "$CLUSTER_FILE_JSON" | jq ". + \$patched | sort" --slurpfile patched "$CLUSTER_FILE_0" > "$CLUSTER_FILE_1"
mv "$CLUSTER_FILE_1" "$CLUSTER_FILE_0"

# HACK convert the json array to multiple yaml documents
for ((i = 0; i < $(jq length "$CLUSTER_FILE_0"); i++)); do
echo "---" >> "$CLUSTER_FILE_1"
jq ".[$i]" "$CLUSTER_FILE_0" | kubectl patch -f - --local -p "{}" --type merge -o yaml >> "$CLUSTER_FILE_1"
done
# HACK convert the array of json objects to multiple yaml documents
json_to_yaml "$CLUSTER_FILE_0" "$CLUSTER_FILE_1"
mv "$CLUSTER_FILE_1" "$CLUSTER_FILE_0"

# Done patching, overwrite original CLUSTER_FILE
# Done patching, overwrite original yaml CLUSTER_FILE
mv "$CLUSTER_FILE_0" "$CLUSTER_FILE" # output is yaml

# Clean up
rm "$CLUSTER_FILE_JSON"
}

function yaml_to_json() {
IN=${1}
OUT=${2}
kubectl patch -f "$IN" --local -p "{}" --type merge -o json | jq '.' -s > "$OUT"
}

function json_to_yaml() {
IN=${1}
OUT=${2}
for ((i = 0; i < $(jq length "$IN"); i++)); do
echo "---" >> "$OUT"
jq ".[$i]" "$IN" | kubectl patch -f - --local -p "{}" --type merge -o yaml >> "$OUT"
done
}
10 changes: 8 additions & 2 deletions hack/e2e/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,13 @@ CLUSTER_TYPE=${CLUSTER_TYPE:-kops}
TEST_DIR=${BASE_DIR}/csi-test-artifacts
BIN_DIR=${TEST_DIR}/bin
SSH_KEY_PATH=${TEST_DIR}/id_rsa
CLUSTER_FILE=${TEST_DIR}/${CLUSTER_NAME}.${CLUSTER_TYPE}.json
CLUSTER_FILE=${TEST_DIR}/${CLUSTER_NAME}.${CLUSTER_TYPE}.yaml
KUBECONFIG=${KUBECONFIG:-"${TEST_DIR}/${CLUSTER_NAME}.${CLUSTER_TYPE}.kubeconfig"}

REGION=${AWS_REGION:-us-west-2}
ZONES=${AWS_AVAILABILITY_ZONES:-us-west-2a,us-west-2b,us-west-2c}
FIRST_ZONE=$(echo "${ZONES}" | cut -d, -f1)
NODE_COUNT=${NODE_COUNT:-3}
INSTANCE_TYPE=${INSTANCE_TYPE:-c4.large}

AWS_ACCOUNT_ID=$(aws sts get-caller-identity --query Account --output text)
Expand All @@ -54,8 +55,10 @@ K8S_VERSION=${K8S_VERSION:-1.20.6}
KOPS_VERSION=${KOPS_VERSION:-1.20.0}
KOPS_STATE_FILE=${KOPS_STATE_FILE:-s3://k8s-kops-csi-e2e}
KOPS_PATCH_FILE=${KOPS_PATCH_FILE:-./hack/kops-patch.yaml}
KOPS_PATCH_NODE_FILE=${KOPS_PATCH_NODE_FILE:-./hack/kops-patch-node.yaml}

EKSCTL_PATCH_FILE=${EKSCTL_PATCH_FILE:-./hack/eksctl-patch.yaml}
EKSCTL_ADMIN_ROLE=${EKSCTL_ADMIN_ROLE:-}

HELM_VALUES_FILE=${HELM_VALUES_FILE:-./hack/values.yaml}

Expand Down Expand Up @@ -111,11 +114,13 @@ if [[ "${CLUSTER_TYPE}" == "kops" ]]; then
"$CLUSTER_NAME" \
"$KOPS_BIN" \
"$ZONES" \
"$NODE_COUNT" \
"$INSTANCE_TYPE" \
"$K8S_VERSION" \
"$CLUSTER_FILE" \
"$KUBECONFIG" \
"$KOPS_PATCH_FILE" \
"$KOPS_PATCH_NODE_FILE" \
"$KOPS_STATE_FILE"
if [[ $? -ne 0 ]]; then
exit 1
Expand All @@ -130,7 +135,8 @@ elif [[ "${CLUSTER_TYPE}" == "eksctl" ]]; then
"$K8S_VERSION" \
"$CLUSTER_FILE" \
"$KUBECONFIG" \
"$EKSCTL_PATCH_FILE"
"$EKSCTL_PATCH_FILE" \
"$EKSCTL_ADMIN_ROLE"
if [[ $? -ne 0 ]]; then
exit 1
fi
Expand Down