Merge upstream 20200713 103725 #83
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of repeating the list of directories and packages to scan for targets like fmt and vet, use variables so we are consistent. Add the version directory to the list. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
This removes redundant settings to make the CI jobs and local jobs more consistent. We can't do that with gosec because the image does not include make. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
This state isn't actually implemented in the code, so before we implement it rename it to "Unmanaged". This reflects the fact that what this state really means is that the Host has been created without attempting to provide credentials. That may have transpired through a discovery process or some other mechanism.
Bring the code into line with the state machine documentation, which says that once the Host is reconciled it will immediately move into one of the states (registering or unmanaged) and not remain with an empty provisioning state. Since the Unmanaged state has not been implemented yet, move unconditionally to Registering. Previously, we waited until the the BMC credentials had at least once been deemed worthy of trying (present, correctly formatted) before the Host would move to the Registering state.
Newly-created Hosts enter the Unmanaged state if no BMC details are provided in the resource Spec. As soon as details are provided - regardless of whether they are complete or correctly formatter - the Host will return to the Registering state. Even if all of the details are later removed, the Host will not return to the Unmanaged state. This is in line with how this state is documented in the state machine diagram, but it was never previously implemented.
When a host is discovered or otherwise created without any BMC data, it now goes into an Unmanaged state, in which buildAndValidateBMCCredentials() is not called. Once a host leaves the unmanaged state (i.e. the BMC address and/or credentials have been supplied at least once), any time the BMC address and/or credentials are missing it is due to a configuration error, not to having newly-discovered a host. Since the unmanaged state handles newly-discovered hosts, in the error handling for buildAndValidateBMCCredentials we can now flag an error if any of the data is missing. This prevents transitions like a Host going back into a "discovered" operational status despite having been provisioned already before the credentials were removed. That should be, and now is, treated as an error.
Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Fixing a couple of variable declarations that were being complained about by ``make lint``. Edit: Updated the Testing.MD doc to add references to the hack scripts. Addresses: metal3-io#575
Replace provisioning interface var
Update testing documentation with tool installation directions
|
/assign @honza |
|
/test unit |
clean up makefile and CI scripts
Implement an Unmanaged (discovered) state (2)
Return the correct error if we fail to update the status subresource or remove status annotation. Remove the status annotation even when it contains data that cannot be unmarshalled correctly.
Do not log reconcile attempts for paused hosts. This keeps the logs quieter for debugging. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
|
Going to wait for couple more PRs upstream to merge before releasing the hold. |
openshift-ci-robot
added
the
do-not-merge/hold
…andling Fix error handling reading status annotation
do not log for paused hosts
|
/hold we need to merge openshift/cluster-api-provider-baremetal#85 before this |
|
/hold clear |
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/retest |
|
openshift/cluster-api-provider-baremetal#85 merged and CI passed so I think this is good to go now /approve |
openshift-ci-robot
added
the
approved
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hardys, honza, kirankt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.