[WIP]✨ integrate AzureMachine with AzureManagedControlPlane #822
Conversation
k8s-ci-robot
added
release-note
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
area/provider/azure
k8s-ci-robot
added
sig/cluster-lifecycle
|
I think if I drop the privilege on the kubeconfig I supply, that will drop the role for node join. So I probably just need an authenticated, non-admin kubeconfig. I can create this ahead of time in the AzureManagedControlPlaneReconciler w a remote client. |
|
|
||
| // AzureManagedControlPlane only works with AzureManagedCluster | ||
| var isManaged bool | ||
| clusterDescriberRef := cluster.Spec.InfrastructureRef |
There was a problem hiding this comment.
feeling the downside of making AzureManagedCluster a no-op right about now...(we could just use infra ref otherwise)
alexeldeib
force-pushed
the
ace/subnet
branch
3 times, most recently
from
ba65adf to
24a4fdd
Compare
k8s-ci-robot
removed
the
needs-rebase
|
@alexeldeib: The following tests 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. |
| // ClusterDescriber is an interface which can get common Azure Cluster information | ||
| // ClusterDescriber describe a Kubernetes resource with full | ||
| // serializability, object meta and type meta, plus can describe enough | ||
| // data to build an Azure cluster | ||
| type ClusterDescriber interface { |
There was a problem hiding this comment.
@CecileRobertMichon any feelings about this? how do you see scope evolving long term for a use case like this?
| } | ||
|
|
||
| // GetAzureDNSZoneForEnvironment returnes the DNSZone to be used with the | ||
| // cloud environment, the default is the public cloud |
There was a problem hiding this comment.
we don't need this function even as-is, the logic here is equivalent
| @@ -99,6 +106,109 @@ type AzureManagedControlPlaneList struct { | |||
| Items []AzureManagedControlPlane `json:"items"` | |||
| } | |||
|
|
|||
| func (c *AzureManagedControlPlane) ManagedResourceGroup() string { | |||
There was a problem hiding this comment.
CRD types now implement clusterDescriber directly, rather than scope
| @@ -126,6 +126,12 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { | |||
| HardwareProfile: &compute.HardwareProfile{ | |||
| VMSize: compute.VirtualMachineSizeTypes(vmSpec.Size), | |||
| }, | |||
| DiagnosticsProfile: &compute.DiagnosticsProfile{ | |||
There was a problem hiding this comment.
These are all great changes but it's too much for one PR, can you please split 1) vm diagnostics 2) NetworkDescriber/ClusterDescriber changes 3) AzureManagedControlPlane etc. in multiple PRs?
There was a problem hiding this comment.
yeah, i'm mostly trying to see if I can get it working first // share some of the changes to patterns that have been helpful trying to make this work for different scenarios. I definitely won't try to merge this as one PR, and I found a few small bits here and there to merge as one-off fixes besides the main functional stuff.
There was a problem hiding this comment.
also vm diagnostics is hardcoded, that was me debugging. once I figure out which order to tackle all these properly I can assign the relevant issues.
|
|
||
| // NetworkDescriber describes the vnet and subnet configuration for a cluster | ||
| // abstracted because it is implemented by managed and unmanaged cluster. | ||
| type NetworkDescriber interface { |
There was a problem hiding this comment.
I have the same thing on my local branch :)
|
/close |
|
@alexeldeib: Closed this PR. In response to this:
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. |
What this PR does / why we need it:
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 #
Special notes for your reviewer:
This joins nodes successfully, but they show up as masters (i guess because of admin config? or maybe something aks side?) and Azure CNI doesn't work yet.
TODOs:
Release note: