-
Notifications
You must be signed in to change notification settings - Fork 192
Conversation
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
//usebom:sandbox/v1.5.0-zshippable/379567558421927594/tkg-compatibility |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
//usebom:sandbox/v1.5.0-zshippable/379670967577783610/tkg-compatibility |
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.
A few mostly minor comments. Otherwise looks good!
A change like this (transitioning between Cluster API versions) can only be done in one big bang, so kudos for the hard work!
Appreciate the amount of effort that went into this. Let's resolve the comments and get this in!
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.
Looks good overall. Minor comments.
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
//usebom:sandbox/v1.5.0-zshippable/379911975624294967/tkg-compatibility |
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.
Thank you for the changes, huge effort!! The changes look good to me with one comment.
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
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.
Just a couple of minor, non-blocking comments. Thanks for the changes, this is a lot of effort!
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
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.
Reviewed addons component. LGTM
Cluster Generation A/B Results: |
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.
Looks good!
Cluster Generation A/B Results: |
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.
LGTM. I think the changes from 0.7.11 made it into 1.0.1 is that your understanding as well? If not, we will have a regression from 1.4.1 to 1.5
@@ -139,7 +144,6 @@ spec: | |||
- diskSizeGB: 256 | |||
lun: 0 | |||
nameSuffix: etcddisk | |||
location: ${AZURE_LOCATION} |
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.
I noticed that this location
property has been removed from the various AzureMachineTemplate in our ytt templates. I assume this means that it isn't need (should be provided in the MachineDeployment anyway)
- change addons-manager to adopt cluster-api v1beta1 - bump controller-runtime version to align with cluster-api - fix the addons integ tests by updating the way how kubeconfig is generated. This change is required to make latest envtest work. Signed-off-by: Daniel Guo <[email protected]>
Cluster Generation A/B Results: |
What this PR does / why we need it
--metrics-addr
to--metrics-bind-addr
and--leader-election
to--leader-elect
to align with cluster-api (Addons controller support for cluster api v1beta1 #654 (comment))--watching-namespace
(⚠️ Remove clusterctl --watching-namespace kubernetes-sigs/cluster-api#4666)We will need to do this in order to bump k8s version to 1.22. Kubernetes 1.22 will not be capable of being lifecycled by v1alpha3 components.
Which issue(s) this PR fixes
Fixes #936, #654
Describe testing done for PR
Release note
PR Checklist
Additional information
Special notes for your reviewer