Skip to content
This repository has been archived by the owner on Nov 20, 2024. It is now read-only.

COSI-15: Add bucket creation E2E tests and cleanup #12

Closed

Conversation

anurag4DSB
Copy link
Collaborator

@anurag4DSB anurag4DSB commented Nov 6, 2024

Based on PR: #11
Add bucket creation E2E tests and cleanup for tests

@anurag4DSB anurag4DSB marked this pull request as ready for review November 6, 2024 08:45
@anurag4DSB anurag4DSB force-pushed the feature/COSI-15-add-bucket-creation-e2e-tests branch 2 times, most recently from f672fba to f89855f Compare November 6, 2024 09:37
@anurag4DSB
Copy link
Collaborator Author

Added a commit to improve kube error handling catch.

@@ -5,7 +5,7 @@ metadata:
namespace: default
type: Opaque
stringData:
COSI_S3_ACCESS_KEY_ID: accessKey1 # Plain text access key
COSI_S3_SECRET_ACCESS_KEY: verySecretKey1 # Plain text secret key
COSI_S3_ACCESS_KEY_ID: PBUOB68AVF39EVVAFNFL # Plain text access key, generated in the CI
Copy link
Collaborator Author

@anurag4DSB anurag4DSB Nov 8, 2024

Choose a reason for hiding this comment

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

We will use github variables later for these, for now keeping it simple
to be expanded in https://scality.atlassian.net/browse/OS-882

log_and_run echo "Creating account in Vault container..."
CONTAINER_ID=$(docker ps -qf "name=s3_and_iam_deployment-iam-1")
log_and_run docker exec "$CONTAINER_ID" sh -c "ADMIN_ACCESS_KEY_ID=D4IT2AWSB588GO5J9T00 ADMIN_SECRET_ACCESS_KEY=UEEu8tYlsOGGrgf4DAiSZD6apVNPUWqRiPG0nTB6 ./node_modules/vaultclient/bin/vaultclient create-account --name cosi-account --email [email protected]"
log_and_run docker exec "$CONTAINER_ID" sh -c "ADMIN_ACCESS_KEY_ID=D4IT2AWSB588GO5J9T00 ADMIN_SECRET_ACCESS_KEY=UEEu8tYlsOGGrgf4DAiSZD6apVNPUWqRiPG0nTB6 ./node_modules/vaultclient/bin/vaultclient generate-account-access-key --name=cosi-account --accesskey=PBUOB68AVF39EVVAFNFL --secretkey=P+PK+uMB9spUc21huaQoOexqdJoV00tSnl+pc7t7"
Copy link
Collaborator Author

@anurag4DSB anurag4DSB Nov 8, 2024

Choose a reason for hiding this comment

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

We will use github variables later for these, for now keeping it simple
to be expanded in https://scality.atlassian.net/browse/OS-882

Comment on lines +41 to +42
aws_access_key_id = PBUOB68AVF39EVVAFNFL
aws_secret_access_key = P+PK+uMB9spUc21huaQoOexqdJoV00tSnl+pc7t7
Copy link
Collaborator Author

@anurag4DSB anurag4DSB Nov 8, 2024

Choose a reason for hiding this comment

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

We will use github variables later for these, for now keeping it simple
to be expanded in https://scality.atlassian.net/browse/OS-882

Comment on lines +62 to +63
COSI_S3_ACCESS_KEY_ID: PBUOB68AVF39EVVAFNFL
COSI_S3_SECRET_ACCESS_KEY: P+PK+uMB9spUc21huaQoOexqdJoV00tSnl+pc7t7
Copy link
Collaborator Author

@anurag4DSB anurag4DSB Nov 8, 2024

Choose a reason for hiding this comment

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

We will use github variables later for these, for now keeping it simple
to be expanded in https://scality.atlassian.net/browse/OS-882

@anurag4DSB anurag4DSB force-pushed the feature/COSI-14-add-iam-s3-logging-and-cleanup-in-CI branch from d915e23 to bdcec3e Compare November 8, 2024 14:41
@anurag4DSB anurag4DSB force-pushed the feature/COSI-15-add-bucket-creation-e2e-tests branch from f89855f to 16f0ca6 Compare November 8, 2024 14:42
@anurag4DSB
Copy link
Collaborator Author

re-based with origin

Base automatically changed from feature/COSI-14-add-iam-s3-logging-and-cleanup-in-CI to main November 8, 2024 14:46

# Step 1: Create Account in Vault
log_and_run echo "Creating account in Vault container..."
CONTAINER_ID=$(docker ps -qf "name=s3_and_iam_deployment-iam-1")
Copy link

Choose a reason for hiding this comment

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

Just a nit. It seems like the container name is static so you could use that instead of retrieving the container id for exec Might be some complications I'm not seeing though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I used to do that, but then got hurt so many times.
But makes sense here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I want to create an action for this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

}

log_and_run echo "Removing COSI driver manifests and namespace..."
log_and_run kubectl delete -k . || echo "COSI driver manifests not found." | tee -a "$LOG_FILE"

Choose a reason for hiding this comment

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

I am not sure the redirection works the way you indended it to due to priority of operators: here the tee only receives the output of the echo command, not the log_and_run output. If you want to redirect both commands you'd need to group them like

{ log_and_run ... || echo "error..."; } | tee -a "$LOG_FILE"

And if you also want stderr to be redirected:

{ log_and_run ... || echo "error..."; } 2>&1 | tee -a "$LOG_FILE"

The same applies to other commands below.

If you actually intend to only redirect the echo, then it can make sense to group with { echo .. | tee; } just for clarity.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

its meant for the latter.
added in 1e58730

log_and_run kubectl patch bucket "$BUCKET_NAME" -p '{"metadata":{"finalizers":[]}}' --type=merge || echo "Finalizers not found for bucket: $BUCKET_NAME" | tee -a "$LOG_FILE"
done

log_and_run echo "Deleting Bucket Claim and Bucket Class..."

Choose a reason for hiding this comment

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

Very minor, just to avoid surprises in case one command fails, to not wrongly assume that the other was executed:

Suggested change
log_and_run echo "Deleting Bucket Claim and Bucket Class..."
log_and_run echo "Deleting Bucket Class and Bucket Claim..."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed in 1e58730

.github/scripts/e2e_test_bucket_creation.sh Show resolved Hide resolved
COSI_S3_ACCESS_KEY_ID: PBUOB68AVF39EVVAFNFL
COSI_S3_SECRET_ACCESS_KEY: P+PK+uMB9spUc21huaQoOexqdJoV00tSnl+pc7t7
COSI_S3_ENDPOINT: http://$HOST_IP:8000
COSI_S3_REGION: us-west-1

Choose a reason for hiding this comment

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

In .aws/config you used us-east-1. Does it make any difference ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No it doesn't make a difference

@anurag4DSB
Copy link
Collaborator Author

Closing this in favor of scality/cosi-driver#1
A new repo is needed so that DX can track it; the old repository was forked, and forked repository data collection is not supported by DX yet.

@anurag4DSB anurag4DSB closed this Nov 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants