Skip to content
This repository was archived by the owner on Sep 29, 2021. It is now read-only.

Disable node discovery by default#67

Merged
metal3-io-bot merged 1 commit into
metal3-io:masterfrom
dtantsur:no-discovery
Nov 16, 2020
Merged

Disable node discovery by default#67
metal3-io-bot merged 1 commit into
metal3-io:masterfrom
dtantsur:no-discovery

Conversation

@dtantsur
Copy link
Copy Markdown
Member

Discovery of new nodes requires explicit support from BMO, which
does not seem to exist, nor is on the roadmap. Having discovery
enabled may cause hard to debug situations when an accidentally
booted IPA results in a new node added.

An option is left to enable discovery for testing purposes or
for future implementation on the BMO side.

Discovery of new nodes requires explicit support from BMO, which
does not seem to exist, nor is on the roadmap. Having discovery
enabled may cause hard to debug situations when an accidentally
booted IPA results in a new node added.

An option is left to enable discovery for testing purposes or
for future implementation on the BMO side.
@metal3-io-bot metal3-io-bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 16, 2020
@dtantsur
Copy link
Copy Markdown
Member Author

/assign @maelk

Do you folks use this feature? We do not, and it seems to cause problems from time to time.

/cc @hardys
/test-integration

@hardys
Copy link
Copy Markdown
Member

hardys commented Nov 16, 2020

AFAIK we don't yet have any support for discovery at the BMO level - it has been discussed in metal3-io/baremetal-operator#41 and metal3-io/metal3-docs#98 but I'm not aware of any plans to complete that work in the near future.

It might be helpful to explain the motivation here, which is that in some circumstances it's possible for IPA to boot and create a port/node in Ironic before inspection is triggered by metal3, then we get a "Port already exists" error which is sometimes confusing for users.

If this change of config helps with that I'm in favor of disabling discovery for now, then if/when we add discovery to BMO we can figure out a smarter way to deal with these kinds of edge cases.

@dhellmann what are your thoughts?

@metal3-io-bot
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtantsur, elfosardo

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

@elfosardo
Copy link
Copy Markdown
Member

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2020
@metal3-io-bot metal3-io-bot merged commit 93494b2 into metal3-io:master Nov 16, 2020
@dhellmann
Copy link
Copy Markdown
Member

What effect does this have on inspection? If a host PXE boots the IPA image will inspection still run automatically? Will the data be saved?

@dtantsur
Copy link
Copy Markdown
Member Author

What effect does this have on inspection?

Inspection can still be requested explicitly, but won't happen accidentally.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants