search

scylladb / scylla-machine-image

Apache License 2.0
19 stars 26 forks source link

packer: share AMIs with the entire aws scylla organization #518

Closed benipeled closed 5 months ago

benipeled commented 6 months ago

With this change, AMIs wil be shared with the entire aws scylla org (instead of specific accounts) to allow all accounts to use the AMI before it is promoted + avoid the need to maintain a list of accounts that require AMI access,

This is about the AMI, on another change on pkg we need to adjust the change_snapshot_permissions() function to loop through all the org account

Ref https://github.com/scylladb/scylla-pkg/issues/3529

benipeled commented 6 months ago

Verified with https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/next-machine-image/293/

yaronkaikov commented 6 months ago

AFAIK the cloud needs those images on the enterprise branch for testing, I don't see a reason to backport this.

benipeled commented 6 months ago

AFAIK the cloud needs those images on the enterprise branch for testing, I don't see a reason to backport this.

Yes, and they probably need it for future releases only but since it's a minor trivial change we'll backport it

yaronkaikov commented 6 months ago

AFAIK the cloud needs those images on the enterprise branch for testing, I don't see a reason to backport this.

Yes, and they probably need it for future releases only but since it's a minor trivial change we'll backport it

We should backport if it's needed or if it's improved something (even if it's a minor and there is no risk). Let's verify before if it's needed for releases. Thanks

benipeled commented 6 months ago

AFAIK the cloud needs those images on the enterprise branch for testing, I don't see a reason to backport this.

Yes, and they probably need it for future releases only but since it's a minor trivial change we'll backport it

We should backport if it's needed or if it's improved something (even if it's a minor and there is no risk). Let's verify before if it's needed for releases. Thanks

Yes it's an improvement and I'm not sure what verification you're looking for here, In any case, let's please avoid a long discussion about something so minor that at the end will effect/help nothing

yaronkaikov commented 6 months ago

AFAIK the cloud needs those images on the enterprise branch for testing, I don't see a reason to backport this.

Yes, and they probably need it for future releases only but since it's a minor trivial change we'll backport it

We should backport if it's needed or if it's improved something (even if it's a minor and there is no risk). Let's verify before if it's needed for releases. Thanks

Yes it's an improvement and I'm not sure what verification you're looking for here, In any case, let's please avoid a long discussion about something so minor that at the end will effect/help nothing

My point exactly to will help nothing :-)