-
Notifications
You must be signed in to change notification settings - Fork 114
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
Updating S3 configuration completion #52
Updating S3 configuration completion #52
Conversation
/hold |
@jupierce watch this space for AWS S3 storage setup for the registry. |
/hold cancel |
if len(d.Config.Region) == 0 { | ||
d.Config.Region = s3cfg.Storage.S3.Region | ||
} | ||
|
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.
comment on the bucket creation logic below:
You need to ensure that on re-sync, we are making sure the bucket exists. That means a few things:
- Adding a condition on the registry resource that indicates when we have successfully created the bucket (we should not be deploying the registry until we are sure we created the bucket)
- a mechanism for ensuring that if the S3 bucket name changed, the condition is cleared until we create the new bucket.
this can be a follow up but i don't consider the s3 feature complete until we have these things.
/retest |
/retest |
/retest |
3 similar comments
/retest |
/retest |
/retest |
/retest |
1 similar comment
/retest |
@bparees ptal |
@bparees I'm sad to hear that. ok, I will spend more time with rebase. as you wish. |
yes i feel badly about it, thank you for understanding. |
No. I do not understand, but it does not matter. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, coreydaley The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No description provided.