Skip to content

Conversation

@ingvagabund
Copy link
Member

What this PR does / why we need it:

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.

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:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:


…de 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.
@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 17, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: ingvagabund

If they are not already assigned, you can assign the PR to them by writing /assign @ingvagabund in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ingvagabund
Copy link
Member Author

ingvagabund commented May 17, 2019

/hold

to give space for discussion

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 17, 2019
@ingvagabund
Copy link
Member Author

Closed in favor of #35

@ingvagabund ingvagabund closed this Jun 4, 2019
@ingvagabund ingvagabund deleted the always-fall-to-machine-node-role branch June 4, 2019 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants