OCPBUGS-18658: Unify agent install-complete with installer #7678
Conversation
openshift-ci-robot
added
jira/severity-important
|
@rwsu: This pull request references Jira Issue OCPBUGS-18658, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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/test-infra repository. |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@rwsu: This pull request references Jira Issue OCPBUGS-18658, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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/test-infra repository. |
openshift-ci
bot
requested review from
patrickdillon,
pawanpinjarkar and
mhanss
|
[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 |
rwsu
force-pushed
the
OCPBUGS-18658
branch
2 times, most recently
from
6c866b0 to
42b48a1
Compare
|
/retest-required |
1 similar comment
|
/retest-required |
| @@ -93,25 +86,24 @@ func newWaitForInstallCompleteCmd() *cobra.Command { | |||
| ctx := context.Background() | |||
| cluster, err := agentpkg.NewCluster(ctx, assetDir) | |||
| if err != nil { | |||
| logrus.Exit(exitCodeBootstrapFailed) | |||
| logrus.Exit(command.ExitCodeBootstrapFailed) | |||
There was a problem hiding this comment.
On a second though/alternative proposal, instead of modifying the rest of the code to unify agent's newWaitForInstallCompleteCmd(), what about using directly installer's newWaitForInstallCompleteCmd()?
There was a problem hiding this comment.
Intriguing idea. There is a slight difference with agent wait-for install-complete. Inside it, it calls agentpkg.WaitForBootstrapComplete. In addition to kubeapi, agent bootstrap-complete also looks queries the assisted-service rest-api for additional status information like printing out the host validations. I don't think we want to lose this additional information by switching to installer's newWaitForInstallCompleteCmd().
There was a problem hiding this comment.
Considering Zane's input, I don't think we can remove the bootstrap-complete from the agent's install-complete because it will break existing user's expectations. Therefore we cannot use the installer's newWaitForInstallCompleteCmd() directly.
|
/retest-required |
|
/test e2e-agent-compact-ipv4 |
openshift-merge-robot
added
the
needs-rebase
openshift-merge-robot
removed
the
needs-rebase
openshift-merge-robot
added
the
needs-rebase
openshift-merge-robot
removed
the
needs-rebase
Removed custom agent wait-for install-complete code. Moved installer WaitForInstallComplete function from cmd/openshift-install/main to cmd/openshift-install/command so that the function can be made public. Modified agent.newWaitForInstallCompleted() to use the common WaitForInstallComplete function. The benefit of moving agent over to the common WaitForInstallComplete function is that the common function has a step to wait for operators to be in a stable state before calling the cluster installation complete.
|
/test e2e-aws-ovn |
|
/lgtm |
|
/retest-required |
1 similar comment
|
/retest-required |
|
@rwsu: 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-sigs/prow repository. I understand the commands that are listed here. |
|
/retest-required |
Removed custom agent wait-for install-complete code.
Moved installer WaitForInstallComplete function from cmd/openshift-install/main to cmd/openshift-install/command so that the function can be made public.
Modified agent.newWaitForInstallCompleted() to use the common WaitForInstallComplete function.
The benefit of moving agent over to the common WaitForInstallComplete function is that the common function has a step to wait for operators to be in a stable state before calling the cluster installation complete.