openshift / oc

The OpenShift Command Line, part of OKD
https://www.openshift.org
Apache License 2.0
196 stars 377 forks source link

WRKLDS-1368: define node-image commands structure #1808

Closed andfasano closed 2 months ago

andfasano commented 3 months ago

This initial PR defines just the initial high level structure for the new add nodes commands (see epic https://issues.redhat.com/browse/WRKLDS-937 for more details), with no flags and no implementation. The main goal of this patch is to converge on the commands code organizations and naming (flags and implementation will followup in subsequent PRs).

Layout will look like:

$ oc adm node-image
The subcommands allow you to create an ISO image to be used for adding the desired nodes to an OpenShift cluster, and
also to monitor the process.

Available Commands:
  create        Create an ISO image for booting the nodes to be added to the target cluster
  monitor       Monitor the process of adding new nodes to an OpenShift cluster

Use "oc adm node-image <command> --help" for more information about a given command.
openshift-ci-robot commented 3 months ago

@andfasano: This pull request references WRKLDS-1368 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.17.0" version, but no target version was set.

In response to [this](https://github.com/openshift/oc/pull/1808): >This initial PR defines just the initial high level structure for the new add nodes commands (see epic https://issues.redhat.com/browse/WRKLDS-937 for more details), with no flags and no implementation. >The main goal of this patch is to converge on the commands code organizations and naming (flags and implementation will followup in subsequent PRs) Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Foc). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci-robot commented 3 months ago

@andfasano: This pull request references WRKLDS-1368 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.17.0" version, but no target version was set.

In response to [this](https://github.com/openshift/oc/pull/1808): >This initial PR defines just the initial high level structure for the new add nodes commands (see epic https://issues.redhat.com/browse/WRKLDS-937 for more details), with no flags and no implementation. >The main goal of this patch is to converge on the commands code organizations and naming (flags and implementation will followup in subsequent PRs). > >Layout will look like: > >``` >$ oc adm add-nodes >The subcommands allow you to create an ISO image to be used for adding the desired nodes to an OpenShift cluster, and >also to monitor the process. > >Available Commands: > create Create an ISO image for booting the nodes to be added to the target cluster > monitor Monitor the process of adding new nodes to an OpenShift cluster >``` Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Foc). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci-robot commented 2 months ago

@andfasano: This pull request references WRKLDS-1368 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.17.0" version, but no target version was set.

In response to [this](https://github.com/openshift/oc/pull/1808): >This initial PR defines just the initial high level structure for the new add nodes commands (see epic https://issues.redhat.com/browse/WRKLDS-937 for more details), with no flags and no implementation. >The main goal of this patch is to converge on the commands code organizations and naming (flags and implementation will followup in subsequent PRs). > >Layout will look like: > >``` >$ oc adm node-image >The subcommands allow you to create an ISO image to be used for adding the desired nodes to an OpenShift cluster, and >also to monitor the process. > >Available Commands: > create Create an ISO image for booting the nodes to be added to the target cluster > monitor Monitor the process of adding new nodes to an OpenShift cluster > >Use "oc adm node-image --help" for more information about a given command. >``` Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Foc). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
andfasano commented 2 months ago

/assign @ingvagabund

ardaguclu commented 2 months ago

I've dropped some comments but overall looks good to me. Thank you

ingvagabund commented 2 months ago

/hold

until at least one (preferably both) of the subcommands is/are introduced. We don't wanna merge empty skeletons.

andfasano commented 2 months ago

/hold

until at least one (preferably both) of the subcommands is/are introduced. We don't wanna merge empty skeletons.

The work for the create command is currently in progress here https://github.com/openshift/oc/pull/1819. The current PR does not link the skeleton commands to oc adm, so the current code will not be visible

openshift-ci[bot] commented 2 months ago

@andfasano: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-ovn-upgrade 542cef4a44025ba40d249e11f258115968184eef link true /test e2e-aws-ovn-upgrade
ci/prow/e2e-aws-ovn-serial 542cef4a44025ba40d249e11f258115968184eef link true /test e2e-aws-ovn-serial

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
racedo commented 2 months ago

Re experimental vs GA, we are now in a developer preview in OpenShift 4.16, which can be considered experimental. The feature is expected as a GA after this dev preview, and we aim to support it as such if all goes to plan during this dev cycle. I'd suggest to please remove the "experimental" note in the description to avoid confusion. Many thanks!

andfasano commented 2 months ago

/hold until at least one (preferably both) of the subcommands is/are introduced. We don't wanna merge empty skeletons.

The work for the create command is currently in progress here #1819. The current PR does not link the skeleton commands to oc adm, so the current code will not be visible

Hi @ingvagabund , the first subcommand PR is now introduced https://github.com/openshift/oc/pull/1819, can we proceed with this PR (since it's the base for 1819)? Thanks

openshift-ci[bot] commented 2 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: andfasano, pawanpinjarkar, rwsu Once this PR has been reviewed and has the lgtm label, please assign soltysh for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/openshift/oc/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
andfasano commented 2 months ago

Included in https://github.com/openshift/oc/pull/1819