AGENT-863: node-joiner cluster script#8242
AGENT-863: node-joiner cluster script#8242openshift-merge-bot[bot] merged 9 commits intoopenshift:masterfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@andfasano: This pull request references AGENT-863 which is a valid jira issue. DetailsIn 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. |
andfasano
force-pushed
the
agent-day2-cluster-script
branch
from
489e176 to
469c264
Compare
zaneb
left a comment
zaneb
left a comment
There was a problem hiding this comment.
This is great. How close are we to having something similar for the wait-for command?
docs/user/agent/node-joiner.sh
Outdated
There was a problem hiding this comment.
It looks like there's an opportunity here to modify the command to allow separate input and output directories, and have some sort of built-in signalling/waiting mechanism.
There was a problem hiding this comment.
Is there really a need for having separate folders? Usually we kept the output in the same assets folder. Adding anyhow the file touch to the code, to have a simpler command
There was a problem hiding this comment.
It's not necessary, but if the go command did what you actually wanted then you wouldn't need to write a shell script here, so that's why I said it's an opportunity 🙂
Any code that has to be built in to oc (i.e. the contents of this script) is very hard to change because you have basically no control/insight of what version the user uses.
Even putting this shell script into the container image would be better than having it here.
openshift-merge-robot
added
the
needs-rebase
andfasano
force-pushed
the
agent-day2-cluster-script
branch
from
6619444 to
0db9133
Compare
openshift-merge-robot
removed
the
needs-rebase
docs/user/agent/node-joiner.sh
Outdated
There was a problem hiding this comment.
I think here we should check the KUBECONFIG environment variable is specified.
There was a problem hiding this comment.
well, in any case the oc command is going to tell that?
andfasano
force-pushed
the
agent-day2-cluster-script
branch
from
f094417 to
a86f74b
Compare
andfasano
force-pushed
the
agent-day2-cluster-script
branch
from
a86f74b to
70d4619
Compare
docs/user/agent/node-joiner.sh
Outdated
There was a problem hiding this comment.
It's not necessary, but if the go command did what you actually wanted then you wouldn't need to write a shell script here, so that's why I said it's an opportunity 🙂
Any code that has to be built in to oc (i.e. the contents of this script) is very hard to change because you have basically no control/insight of what version the user uses.
Even putting this shell script into the container image would be better than having it here.
docs/user/agent/add-nodes.md
Outdated
There was a problem hiding this comment.
Suggest a separate directory for this stuff. There are going to be 3 files that go together on a topic separate to the rest of the agent stuff, so it could be confusing to have them all mixed together.
There was a problem hiding this comment.
Dedicated folder sounds good to me, but I read two files only (add-nodes.md and node-joiner.sh)?
images/installer/Dockerfile.ci
Outdated
There was a problem hiding this comment.
A thought that just occurred to me: if the cluster has FIPS enabled we want to run this in FIPS mode, with the dynamically-linked binary. This is safe because we're always running it in our own container, which will have the right deps.
So I think we actually want to add this to the baremetal-installer container, not this one. That's actually a better fit because it contains binaries for only one CPU architecture.
Also we'll want to set CGO_ENABLED=1 in hack/build-node-joiner.sh.
Finally, we'll want to set the fips=1 karg in the ISO when FIPS is enabled on the cluster, if we don't already (suggest you raise a separate ticket for that).
There was a problem hiding this comment.
I will create a new ticket for the FIPS support in the new GA epic
docs/user/agent/node-joiner.sh
Outdated
There was a problem hiding this comment.
This is as much for UPI users as agent users, so maybe we should just name this node.x86_64.iso?
We should think about how we will handle the architecture not being known (and potentially there being multiple in the future). Not needed for now, but it is coming.
Also PXE, presumably there will be a tarfile or something for that.
It may be time to create an Epic for GA-ing this feature, and start creating stories under it to track all the known work items that we'll need in addition to native oc support.
andfasano
force-pushed
the
agent-day2-cluster-script
branch
from
70d4619 to
6004a83
Compare
There was a problem hiding this comment.
RBAC for Secrets in all namespaces is something to tidy up at some point.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zaneb The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
…th the required dependencies (nmstate and oc)
command now generates directly an exit code file. random namespace generation. config file name customizable.
Co-authored-by: Richard Su <rwsu@redhat.com>
andfasano
force-pushed
the
agent-day2-cluster-script
branch
from
5519a6d to
89bcfdf
Compare
docs/user/agent/node-joiner.sh
Outdated
There was a problem hiding this comment.
Nit
| # Runt the node-joiner pod to generate the ISO | |
| # Run the node-joiner pod to generate the ISO |
|
/retest-required |
|
@andfasano: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/hold Revision 89bcfdf was retested 3 times: holding |
openshift-ci
bot
added
the
do-not-merge/hold
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
..and follow along with openshift/installer#8242
..and follow along with openshift/installer#8242
Derived from a similar script by Andrea Fasano to generate the add-nodes ISO. openshift#8242 This script tweaks it and creates a node-joiner-monitor pod to monitor adding nodes to a cluster. Co-authored-by: Andrea Fasano <andfasano@redhat.com>
6 participants
This patch adds to the
installerimage thenode-joinerbinary, along with its required dependencies.It also adds a
node-joiner.shscript to allow running the node-joiner tool within the target cluster to be expanded.Documentation on how to use it it's also provided.