chore: remove deprecated AKS code paths #4040
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4040 +/- ##
==========================================
+ Coverage 73.08% 73.22% +0.14%
==========================================
Files 145 135 -10
Lines 21776 20640 -1136
==========================================
- Hits 15914 15113 -801
+ Misses 4829 4550 -279
+ Partials 1033 977 -56
Continue to review full report at Codecov.
|
| @@ -424,7 +418,7 @@ func (cs *ContainerService) setOrchestratorDefaults(isUpgrade, isScale bool) { | |||
|
|
|||
| // Upgrade scenario: | |||
| // We need to force set EnableRbac to true for upgrades to 1.15.0 and greater if it was previously set to false (AKS Engine only) | |||
| if !a.OrchestratorProfile.KubernetesConfig.IsRBACEnabled() && common.IsKubernetesVersionGe(o.OrchestratorVersion, "1.15.0") && isUpgrade && !cs.Properties.IsHostedMasterProfile() { | |||
| if !a.OrchestratorProfile.KubernetesConfig.IsRBACEnabled() && common.IsKubernetesVersionGe(o.OrchestratorVersion, "1.15.0") && isUpgrade { | |||
There was a problem hiding this comment.
Optionally, we could also drop the common.IsKubernetesVersionGe(o.OrchestratorVersion, "1.15.0") test here. I guess I missed that in #3851.
There was a problem hiding this comment.
Yeah we should do a follow-up workstream just to (1) get rid of the unnecessary version stuff, and (2) automate these versions in tests so we don't have to ever again manually maintain them
| @@ -136,7 +136,7 @@ func (rcc *rotateCertsCmd) run(cmd *cobra.Command, args []string) error { | |||
| if rcc.containerService.Properties.MasterProfile != nil { | |||
| rcc.outputDirectory = path.Join("_output", rcc.containerService.Properties.MasterProfile.DNSPrefix) | |||
| } else { | |||
| rcc.outputDirectory = path.Join("_output", rcc.containerService.Properties.HostedMasterProfile.DNSPrefix) | |||
| return errors.New("can't determine output directory from nil MasterProfile") | |||
There was a problem hiding this comment.
Can we add a unit test for this error path to rotate_certs_test.go? Or at least put a pin in it by creating a GitHub issue.
There was a problem hiding this comment.
Actually I'm going to punt on this one as this whole thing is going to make @jadarsie's life harder and so why not make it even worse.
(mostly) kidding... but tl;dr I'll work with him on rebasing once this lands and then adapting his rewrite of rotate certs to deal with this stuff.
There was a problem hiding this comment.
Added a thumbs down for visual fidelity
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackfrancis, mboersma 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 |
Reason for Change:
This PR removes all deprecated (no longer used by) AKS code paths, in order to make ongoing maintenance of AKS Engine more streamlined.
Issue Fixed:
Credit Where Due:
Does this change contain code from or inspired by another project?
If "Yes," did you notify that project's maintainers and provide attribution?
Requirements:
Notes: