Skip to content
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

added machineclasses CRD for out-of-tree machine controllers #2625

Merged
merged 1 commit into from
Jul 28, 2020

Conversation

MartinWeindel
Copy link
Member

How to categorize this PR?

/area control-plane
/kind enhancement
/priority normal

What this PR does / why we need it:
The CRD of the generic machineclass used by all out-of-tree machine controllers is added to the well-known list of machine CRDs. This avoids that every infrastructure provider using OOT MCM has to deploy it explicitly.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

added machineclasses CRD for out-of-tree machine controllers

@MartinWeindel MartinWeindel requested a review from a team as a code owner July 24, 2020 07:02
@gardener-robot gardener-robot added area/control-plane Control plane related kind/enhancement Enhancement, improvement, extension priority/normal labels Jul 24, 2020
@MartinWeindel
Copy link
Member Author

cc @prashanth26

@prashanth26
Copy link

Hi @MartinWeindel ,

Thanks for the quick change. Also this might not be needed right? - https://github.com/gardener/gardener/blob/f12bb20a834bd14221f8d9a8f36dd70de7d2aa47/extensions/pkg/controller/worker/machine_crds.go#L196-L333. If it's not required, we could remove this as well?

@MartinWeindel
Copy link
Member Author

Hi @MartinWeindel ,

Thanks for the quick change. Also this might not be needed right? -

https://github.com/gardener/gardener/blob/f12bb20a834bd14221f8d9a8f36dd70de7d2aa47/extensions/pkg/controller/worker/machine_crds.go#L196-L333

. If it's not required, we could remove this as well?

Wouldn't it be better to keep the old machine classes as long as there are still providers with in-tree machine controllers?

@rfranzke
Copy link
Member

/invite @prashanth26 @hardikdr

@prashanth26
Copy link

Wouldn't it be better to keep the old machine classes as long as there are still providers with in-tree machine controllers?

Sorry, you are right. I mistook this change to be on the provider-vSphere repo, it's actually on the gardener/extensions. Yes, this old code should exist here. Probably in future, we can get rid of the old machine classes once migrations are done.

rfranzke
rfranzke previously approved these changes Jul 27, 2020
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/lgtm

hardikdr
hardikdr previously approved these changes Jul 27, 2020
Copy link
Member

@hardikdr hardikdr left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks.

@MartinWeindel
Copy link
Member Author

updated description text for provider field as suggested by @prashanth26

Copy link

@prashanth26 prashanth26 left a comment

Choose a reason for hiding this comment

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

/lgtm

@rfranzke rfranzke merged commit 5088386 into gardener:master Jul 28, 2020
@MartinWeindel MartinWeindel deleted the ext-oot-machineclasses-crd branch September 15, 2020 08:30
@gardener-robot gardener-robot added priority/3 Priority (lower number equals higher priority) and removed priority/3 Priority (lower number equals higher priority) labels Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/control-plane Control plane related kind/enhancement Enhancement, improvement, extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants