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

Dead Code Removal https://github.com/gardener/machine-controller-man… #769

Merged
merged 19 commits into from
Jan 27, 2023
Merged

Conversation

elankath
Copy link
Contributor

@elankath elankath commented Jan 19, 2023

Dead Code Removal #622

This commit removes dead and deprecated code from the machine controller manager.

What this PR does / why we need it:
This commit removes dead and deprecated code from the machine controller manager. The machine controller is now launched by individual providers and not by the main MCM. Old Migration code from old provider specific machine classes to harmonized machine class has also been removed..

  • All provider specific types removed from pkg/apis/machine/types.go and api clients regenerated using make generate
  • All migration code from provider specific classes to common MachineClass removed.
  • Driver migration method removed
  • Deprecated MCM flags removed.
  • Removal of provider specific machine classes in kubernetes/crds
  • Removal of provider specific machine class yamls in kubernetes/machine_classes
  • Removal of old node drain code since this is now handled by the machine controller code present in pkg/util/provider/drain/drain.go which is invoked by platform specific providers.
  • Some minor refactoring of routines and constants to adapt to the above.

Which issue(s) this PR fixes:
#622

Special notes for your reviewer:
This change breaks compilation in individual provider repos which need to be updated. Removal of Driver.GenerateMachineClassForMigration needs to be conveyed to other stakeholders.

Release note:

Removal of the following flags (and corresponding fields in associated structs): 'machine-creation-timeout' 'machine-drain-timeout', 'machine-pv-detach-timeout', 'machine-health-timeout=10m', 'machine-safety-apiserver-statuscheck-timeout', 'machine-safety-apiserver-statuscheck-period', 'machine-safety-orphan-vms-period', 'machine-max-evict-retries', 'node-conditions', 'bootstrap-token-auth-extra-groups', 'delete-migrated-machine-class'. The MCM no longer accepts these flags since these are options handled by the Machine Controller invoked by platform specific provider launchers.
Deletion of 'Driver.GenerateMachineClassForMigration'. Providers need to adapt to this.

@elankath elankath requested a review from a team as a code owner January 19, 2023 10:20
@gardener-robot gardener-robot added the kind/api-change API change with impact on API users label Jan 19, 2023
@CLAassistant
Copy link

CLAassistant commented Jan 19, 2023

CLA assistant check
All committers have signed the CLA.

@gardener-robot gardener-robot added needs/second-opinion Needs second review by someone else needs/review Needs review size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) labels Jan 19, 2023
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 19, 2023
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 19, 2023
@himanshu-kun himanshu-kun mentioned this pull request Jan 23, 2023
@himanshu-kun himanshu-kun self-assigned this Jan 25, 2023
Copy link
Contributor

@himanshu-kun himanshu-kun left a comment

Choose a reason for hiding this comment

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

In addition to the changes requested, kindly also remove the in-tree documentation related to how to deploy MCM

The following changes should also be done:
Other files related to doc need to be removed :

pkg/controller/controller_suite_test.go Outdated Show resolved Hide resolved
pkg/controller/controller.go Outdated Show resolved Hide resolved
pkg/controller/controller.go Outdated Show resolved Hide resolved
pkg/controller/controller.go Outdated Show resolved Hide resolved
pkg/controller/controller.go Outdated Show resolved Hide resolved
pkg/controller/machine_safety.go Outdated Show resolved Hide resolved
@gardener-robot gardener-robot added the needs/changes Needs (more) changes label Jan 25, 2023
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 27, 2023
Copy link
Contributor

@himanshu-kun himanshu-kun left a comment

Choose a reason for hiding this comment

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

pkg/controller/controller.go Outdated Show resolved Hide resolved
pkg/util/provider/machinecontroller/controller.go Outdated Show resolved Hide resolved
@himanshu-kun himanshu-kun added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 27, 2023
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 27, 2023
Copy link
Contributor

@himanshu-kun himanshu-kun left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/changes Needs (more) changes needs/review Needs review needs/second-opinion Needs second review by someone else labels Jan 27, 2023
@gardener-robot gardener-robot added needs/second-opinion Needs second review by someone else and removed reviewed/lgtm Has approval for merging labels Jan 27, 2023
@himanshu-kun himanshu-kun added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 27, 2023
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 27, 2023
@himanshu-kun himanshu-kun merged commit 846e688 into gardener:master Jan 27, 2023
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Jan 27, 2023
@himanshu-kun himanshu-kun linked an issue Feb 20, 2023 that may be closed by this pull request
3 tasks
@himanshu-kun himanshu-kun mentioned this pull request Feb 20, 2023
3 tasks
@himanshu-kun himanshu-kun modified the milestone: 2023-Q2 Mar 8, 2023
himanshu-kun added a commit that referenced this pull request Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/api-change API change with impact on API users needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/second-opinion Needs second review by someone else size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove deprecated in-tree code
7 participants