-
Notifications
You must be signed in to change notification settings - Fork 428
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
Add ComputeGallery field and add community galleries support #2277
Add ComputeGallery field and add community galleries support #2277
Conversation
/retest
|
this is the error I'm seeing in logs
:( |
e2e tests are passing again :) |
Downloading helm fails on CI, though it works for me locally, let's try later... |
cc @jsturtevant or @mboersma for a second pair of 👀 lgtm pending openAPI comment |
/lgtm |
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 sounds badly names, plan is different than pub/off/sku:
https://docs.microsoft.com/en-us/rest/api/compute/virtual-machines/create-or-update#plan
is this on prupose or outside of ur control to name this correctly?
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 lgtm, pending one comment about the AzureMarketPlaceImage
CRD.
lgtm pending 2 minor docs comments |
Signed-off-by: Mateusz Gozdek <[email protected]>
To allow embedding it in newly added type for compute galleries. Signed-off-by: Mateusz Gozdek <[email protected]>
Azure has renamed Shared Image Gallery (SIG) to Azure Compute Gallery at some point, so this commit reflects that by introducing new type which also makes use of new ImagePlan struct to include plan information, which makes it share more code with other types and also treats ResourceGroup and SubscriptionID as string pointers as a preparation for adding community galleries support, which do not require those fields to be populated. This is done as a separate step to better highlight what consists of an actual community galleries support. Refs #2267 Signed-off-by: Mateusz Gozdek <[email protected]>
By reusing newly introduced ComputeGallery type fields. Refs #2267 Signed-off-by: Mateusz Gozdek <[email protected]>
Signed-off-by: Mateusz Gozdek <[email protected]>
@invidian: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
/lgtm |
All conversations are resolved. Thanks for reviewing it again :) |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CecileRobertMichon 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 |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds support for creating machines using recently made available in public preview community galleries images, which can be shared between subscriptions and tenants, which makes it easier to share than publishing to marketplace.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #2267
Special notes for your reviewer:
Extracted from #1729, e2e tests are done there. Please let me know what kind of tests would you like to have added in this PR.
TODOs:
Release note: