AGENT-858: Agent day2 ignition services#8093
AGENT-858: Agent day2 ignition services#8093openshift-merge-bot[bot] merged 14 commits intoopenshift:masterfrom
Conversation
|
@andfasano: This pull request references AGENT-858 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. |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
bot
added
the
do-not-merge/work-in-progress
andfasano
force-pushed
the
agent-day2-ignition-services
branch
from
247e237 to
275814a
Compare
andfasano
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
andfasano
force-pushed
the
agent-day2-ignition-services
branch
4 times, most recently
from
c5d4bb9 to
6d34d8e
Compare
|
/retest |
andfasano
force-pushed
the
agent-day2-ignition-services
branch
2 times, most recently
from
71b098a to
45e8797
Compare
andfasano
force-pushed
the
agent-day2-ignition-services
branch
from
45e8797 to
30692fc
Compare
|
/test golint |
rwsu
left a comment
rwsu
left a comment
There was a problem hiding this comment.
/lgtm
In a follow up PR, it would be useful to log out some information about the ISO to help with debugging customer issues: perhaps cluster name and the IP addresses for the nodes being added.
openshift-ci
bot
added
lgtm
openshift-merge-robot
added
the
needs-rebase
andfasano
force-pushed
the
agent-day2-ignition-services
branch
from
ad47230 to
828612c
Compare
- removed role patch as not required - managed error for host install call
andfasano
force-pushed
the
agent-day2-ignition-services
branch
from
479d224 to
cac843c
Compare
openshift-merge-robot
removed
the
needs-rebase
| if [[ "${cluster_id}" = "" ]]; then | ||
| sleep 2 | ||
| fi | ||
| done |
There was a problem hiding this comment.
Do we need a timeout followed by error message here? Alternatively, do we need a status_issue for this step?
There was a problem hiding this comment.
Good question. I think here, the cluster_id should be available fairly quickly as the agent-add-node service has an After dependency on apply-host-config.service, which itself, has a dependency on agent-register-infra.service and it on agent-import-cluster.service. So by the time this script/service runs the cluster_id should be available.
@sadasu What is a status_issue? Could you explain? Thanks!
There was a problem hiding this comment.
Yeah, as per the agent services architecture I'd expect that in case of issue the agent-register-cluster would prevent the other services, the loop here it's just to cope in case of slower execution (similar approach in another scritps)
There was a problem hiding this comment.
@sadasu Please disregard my question. I see status_issue updated below.
|
/lgtm One inline comment that doesn't have to stop progress of PR. |
| if [[ "${cluster_id}" = "" ]]; then | ||
| sleep 2 | ||
| fi | ||
| done |
There was a problem hiding this comment.
Good question. I think here, the cluster_id should be available fairly quickly as the agent-add-node service has an After dependency on apply-host-config.service, which itself, has a dependency on agent-register-infra.service and it on agent-import-cluster.service. So by the time this script/service runs the cluster_id should be available.
@sadasu What is a status_issue? Could you explain? Thanks!
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rwsu 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
|
/lgtm |
|
/hold Revision cac843c was retested 3 times: holding |
openshift-ci
bot
added
the
do-not-merge/hold
|
@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 cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/retest-required |
6 participants
This patch is based on PR #8080 (and also reuses service definitions from PR #8051 thanks @rwsu)
In this patch the Ignition asset is modified to enable the required services in case of add-node workflow