[WIP] OCPEDGE-1505: Enhance Platform none with Fencing Credentials #9521
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@mshitrit: This pull request references OCPEDGE-1505 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test ? |
|
@patrickdillon: The following commands are available to trigger required jobs: The following commands are available to trigger optional jobs: Use In response to this:
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-sigs/prow repository. |
|
/test gofmt golint govet |
|
This LGTM. I will let others with more direct expertise review before tagging approve. |
mshitrit
force-pushed
the
fencing-config-platform-none
branch
from
44858d6 to
30b3f8d
Compare
|
/test gofmt golint govet |
mshitrit
force-pushed
the
fencing-config-platform-none
branch
from
30b3f8d to
76b034c
Compare
|
/test gofmt golint govet |
mshitrit
force-pushed
the
fencing-config-platform-none
branch
from
76b034c to
a4f1ccc
Compare
|
/test gofmt golint govet |
pkg/asset/agent/installconfig.go
Outdated
| case none.Name: | ||
| { | ||
| fieldPath := field.NewPath("Platform", "None") | ||
| allErrs = append(allErrs, nonevalidation.ValidateFencingCredentials(installConfig.Platform.None.FencingCredentials, fieldPath)...) |
There was a problem hiding this comment.
I'm not sure if that's the correct location to trigger the validation.
Since this is the agent package I suspect it might be the wrong spot and should be somewhere around here 🤔
There was a problem hiding this comment.
We will need to call validate, here, but probably not in this PR.
I think your hunch is correct regarding the platformprovisioncheck file.
|
The enhancement mentions a feature gate, You would need to add the equivalent for the baremetal platform and none. I did not see the featuregate in openshift api. A feature gate needs to be first merged to openshift api and then vendored into the installer. |
There was a problem hiding this comment.
Overall I like the general idea, but I'm worried about locking us into the same BMC spec as Baremetal unless we're commiting to offering the same options.
Also, as we discussed this morning, it would be good to verify that the list of credentials is exactly of length 2.
Signed-off-by: Michael Shitrit <[email protected]>
Signed-off-by: Michael Shitrit <[email protected]>
Signed-off-by: Michael Shitrit <[email protected]>
Signed-off-by: Michael Shitrit <[email protected]>
… merged) Signed-off-by: Michael Shitrit <[email protected]>
mshitrit
force-pushed
the
fencing-config-platform-none
branch
from
a4f1ccc to
59856d8
Compare
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
/test gofmt golint govet |
|
@mshitrit: all tests passed! Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
As part of the TNF (Two Nodes Fencing) project we are looking to enhance platform none with fencing credentials.
This is required so users can provide the fencing details as they are mandatory for a TNF cluster.
This change is described in more details in the TNF Enhancement Proposal and is tracked by OCPEDGE-1505 Jira ticket.