Skip to content

Cherry pick commits for namespacing ironic node names#140

Closed
asalkeld wants to merge 4 commits intoopenshift:masterfrom
asalkeld:ironic-host-namespaced-downstream
Closed

Cherry pick commits for namespacing ironic node names#140
asalkeld wants to merge 4 commits intoopenshift:masterfrom
asalkeld:ironic-host-namespaced-downstream

Conversation

@asalkeld
Copy link
Copy Markdown

This also pulls in Zane's small refactor so the code does not diverge too much.
/cc @zaneb

zaneb and others added 4 commits March 31, 2021 11:21
The empty provisioner is a workaround for the fact that the ironic
provisioner cannot handle instantiation when there are no BMC details
available. The change cadeb28 that
added it also had the effect of logging provisioner information on every
reconcile when it was intended to be logged only once, at startup.

Make it the ironic provisioner factory's responsibility to return an
empty Provisioner at runtime when it cannot create an ironic
Provisioner, and do only one-time startup configuration in main.go.

Fixes metal3-io#819
Not every provisioner operation requires BMC access details, so only
create them when needed. This allows us to reach the unmanaged state
using the ironic provisioner when there are no BMC details specified.

This also means that if the BMC details cannot be parsed correctly,
rather than an error that causes constant requeues and is recorded only
in the logs, we will record a registration failure that will be visible
in the UI.
Now that the ironic provisioner only requires BMC details when
registering (which does not occur in the unmanaged state), remove the
empty provisioner workaround.
- Don't search by nodename if it has "~" in it
- if POD_NAMESPACE is "" then use watchNamespace for the leader
@openshift-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: asalkeld

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 31, 2021
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Mar 31, 2021

@asalkeld: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-metal-ipi-ovn-ipv6 82e9454 link /test e2e-metal-ipi-ovn-ipv6

Full PR test history. Your PR dashboard.

Details

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.

@zaneb
Copy link
Copy Markdown
Member

zaneb commented Mar 31, 2021

We'll want whatever the outcome of metal3-io#842 is as well.

Not specific to this PR, but is there a reason that we're cherry-picking stuff rather than just merging from upstream? Some sort of feature freeze I'm not aware of?

@asalkeld
Copy link
Copy Markdown
Author

We'll want whatever the outcome of metal3-io#842 is as well.

Not specific to this PR, but is there a reason that we're cherry-picking stuff rather than just merging from upstream? Some sort of feature freeze I'm not aware of?

I honestly just don't know what the default process is.. I assumed someone would do a periodic merge

@zaneb
Copy link
Copy Markdown
Member

zaneb commented Apr 1, 2021

#139 contains all of these commits

@asalkeld asalkeld closed this Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants