Use authentication for Ironic on baremetal bootstrap host #4256
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
zaneb
force-pushed
the
ironic-basic-auth
branch
7 times, most recently
from
8e03ebd
to
4aaf978
Compare
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
zaneb
force-pushed
the
ironic-basic-auth
branch
from
4aaf978
to
0a54275
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/assign @stbenjam |
|
/approve |
|
/retest |
There was a problem hiding this comment.
My main concern is about the way pkg/types/baremetal/auth works -- I think the asset store managed by the installer in pkg/asset provides the facilities you need, and avoids the concurrency concerns you're working around. More docs are here: https://github.com/openshift/installer/blob/master/docs/design/assetgeneration.md
If you used the same logic as the kubeadmin password (pkg/asset/password), we could even get a file written out that contains the creds for debugging ironic. That file could potentially even be in the clouds.yaml format directly.
One other question: could we get the installer to create the secret for MAO/CBO to use? That would mean the bootstrap and cluster provisioning infrastructures end up using the same password, and the generated file in auth would work for both.
data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template
Outdated
Show resolved
Hide resolved
|
@zaneb: The following tests failed, say
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/test-infra repository. I understand the commands that are listed here. |
zaneb
force-pushed
the
ironic-basic-auth
branch
from
0a54275
to
e9a9fd7
Compare
Done.
Instead of this I added a templated clouds.yaml file on the bootstrap host that we can potentially fetch in dev-scripts.
We could but I don't think it's advisable. The Secret in the cluster is a lot better protected than the one we give to the bootstrap, which gets written to disk in a dozen different places. IMHO it's best if these creds are ephemeral. |
zaneb
force-pushed
the
ironic-basic-auth
branch
2 times, most recently
from
6ac404b
to
782dca9
Compare
zaneb
force-pushed
the
ironic-basic-auth
branch
from
782dca9
to
61b8bde
Compare
|
/retest |
|
metal-ipi job is failing with "Internal Server Error" (presumably from Ironic?) in terraform, but I can't reproduce that locally. |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
The problem trying to get data out of CI is we don't get installer log bundle when terraform fails early, #3927. My proposed fix was rejected, I have to revisit their feedback. It seems likely Ironic is failing, I'll try this on my box if I can figure it out. |
zaneb
force-pushed
the
ironic-basic-auth
branch
from
61b8bde
to
8253be8
Compare
|
So Ironic is failing when using IPv6. I am yet to figure out how (or, indeed, whether) that is linked to the changes in this PR. |
|
So several things combined here to make it fail on recent versions of this PR, even though there's nothing wrong with the code here.
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
zaneb
force-pushed
the
ironic-basic-auth
branch
from
8253be8
to
2a1592a
Compare
|
This should be unblocked by openshift/ironic-image#113 |
openshift-ci-robot
removed
the
do-not-merge/hold
Add an asset to generate the credentials for Ironic on the bootstrap once and share them with the both Terraform and Ignition.
If it becomes necessary to connect to the ironic or inspector services from the command line for debugging purposes, it is useful to have a clouds.yaml file available for the openstack client to use. Generate this file in the correct format on the bootstrap host for convenience.
zaneb
force-pushed
the
ironic-basic-auth
branch
from
2a1592a
to
ef5c494
Compare
|
/retest |
|
@zaneb: The following tests failed, say
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/test-infra repository. I understand the commands that are listed here. |
|
I'm happy with this version! I'm worried a bit about how many ways we're constructing URL's: in bash, go, and then separately in golang templating but I don't think there's much we can do about it. Hopefully our CI is smart enough to catch regressions when inevitably we screw up building IPv6 URL's correctly. /lgtm |
|
@staebler Can you have another look? We'll need someone from installer team to approve this PR. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhellmann, staebler 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 |
openshift-ci-robot
added
the
approved
No description provided.