-
Notifications
You must be signed in to change notification settings - Fork 34
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.
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
Integration tests #15
Conversation
# running by `concurrency: e2e-cluster` option. | ||
concurrency: e2e-cluster | ||
env: | ||
AWS_REGION : "us-east-1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
standard practice is to use us-west-2 for testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, this will require recreating all of the resources in that region (cluster, registry, iam roles); i would suggest using the existing one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jjkr do you have any issues with keeping testing infra in IAD?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IAD is fine by me. The us-west-2 convention is mainly for web services and not strong enough that we need to change resources.
@@ -0,0 +1,158 @@ | |||
module github.com/awslabs/aws-s3-csi-driver/tests/e2e-kubernetes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's our plan to update this file? will it be automated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, but it's unlikely that this will be required, as we will be able to use precompiled test binary after implementing dynamic provisioning (as described here)
I believe lack of dynamic provisioning is the only aspect which distinguishes us from most of the other drivers and stops from using the simple testing approach through the binary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dynamic provisioning will be a ways away for us, what's our plan to update this file in the short term?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manually updating the dependencies to match k8s version:
k8s version v1.28.3
-> dependency version v0.28.3
}, nil | ||
} | ||
|
||
func (v *s3Volume) DeleteVolume(ctx context.Context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're not enabling the ability to delete volumes/buckets, any reason we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is testing code which creates/deletes buckets for tests (usually we just use our personal, but for tests buckets need to be empty)
for clarification: this code won't be ever executed on customer workload / with customer-owned buckets
Add github action, which is triggered by a push to any branch and:
https://quip-amazon.com/TSFyAZbLD6lR/S3-CSI-integration-testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.