From 1fe9608d1d99df37af655f328b0eba06785159c4 Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Fri, 17 May 2019 20:01:11 +0200 Subject: [PATCH] UPSTREAM: : openshift: Always consider all machines to have node role As part of minimizing the number of machine labels needed by actuators to make decisions, we want to remove `machine.openshift.io/cluster-api-machine-role` machine label, which is in Azure actuator used to distinguishes between node and control plane machine. Given openshift actuators do not create control plane machines, it's safe to assume all machines are node machines for now. It's impossible to remove dependency on `machine.openshift.io/cluster-api-machine-role` machine label without removing a bunch of tests relying on control plane machine role. Thus, instead of removing the label, just fallback to node role by default. Thus, eliminating the need to set `machine.openshift.io/cluster-api-machine-role` machine label. --- .../azure/actuators/machine/reconciler.go | 22 +++++++++++++++++-- pkg/cloud/azure/actuators/machine_scope.go | 3 ++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/pkg/cloud/azure/actuators/machine/reconciler.go b/pkg/cloud/azure/actuators/machine/reconciler.go index 3bc2003d140..b0ad2bf32c4 100644 --- a/pkg/cloud/azure/actuators/machine/reconciler.go +++ b/pkg/cloud/azure/actuators/machine/reconciler.go @@ -60,6 +60,10 @@ type Reconciler struct { virtualMachinesSvc azure.Service virtualMachinesExtSvc azure.Service disksSvc azure.Service + + // NOTE(jchaloup): openshift actuators manage node role machines only. + // Set to true outside of the fake reconciler. + alwaysFallbackToNodeRole bool } // NewReconciler populates all the services based on input scope @@ -71,6 +75,10 @@ func NewReconciler(scope *actuators.MachineScope) *Reconciler { virtualMachinesSvc: virtualmachines.NewService(scope.Scope), virtualMachinesExtSvc: virtualmachineextensions.NewService(scope.Scope), disksSvc: disks.NewService(scope.Scope), + + // NOTE(jchaloup): openshift actuators manage node role machines only. + // Set to true outside of the fake reconciler. + alwaysFallbackToNodeRole: true, } } @@ -249,7 +257,12 @@ func (s *Reconciler) isNodeJoin() (bool, error) { return false, errors.Wrapf(err, "failed to retrieve machines in cluster") } - switch set := s.scope.Machine.ObjectMeta.Labels[v1alpha1.MachineRoleLabel]; set { + // NOTE(jchaloup): openshift actuators manage node role machines only. + set := v1alpha1.Node + if !s.alwaysFallbackToNodeRole { + set = s.scope.Machine.ObjectMeta.Labels[v1alpha1.MachineRoleLabel] + } + switch set { case v1alpha1.Node: return true, nil case v1alpha1.ControlPlane: @@ -449,7 +462,12 @@ func (s *Reconciler) createNetworkInterface(ctx context.Context, nicName string) Name: nicName, VnetName: azure.GenerateVnetName(s.scope.Cluster.Name), } - switch set := s.scope.Machine.ObjectMeta.Labels[v1alpha1.MachineRoleLabel]; set { + // NOTE(jchaloup): openshift actuators manage node role machines only. + set := v1alpha1.Node + if !s.alwaysFallbackToNodeRole { + set = s.scope.Machine.ObjectMeta.Labels[v1alpha1.MachineRoleLabel] + } + switch set { case v1alpha1.Node: networkInterfaceSpec.SubnetName = azure.GenerateNodeSubnetName(s.scope.Cluster.Name) case v1alpha1.ControlPlane: diff --git a/pkg/cloud/azure/actuators/machine_scope.go b/pkg/cloud/azure/actuators/machine_scope.go index 5913984d1e0..f9376b1e30d 100644 --- a/pkg/cloud/azure/actuators/machine_scope.go +++ b/pkg/cloud/azure/actuators/machine_scope.go @@ -120,7 +120,8 @@ func (m *MachineScope) Namespace() string { // Role returns the machine role from the labels. func (m *MachineScope) Role() string { - return m.Machine.Labels[v1alpha1.MachineRoleLabel] + // NOTE(jchaloup): openshift actuators manage node role machines only. + return v1alpha1.Node } // Location returns the machine location.