upbound / up

The @upbound CLI
Apache License 2.0
51 stars 41 forks source link

Move `ctp connect` to `ctp connector install` #389

Closed tnthornton closed 10 months ago

tnthornton commented 10 months ago

Description of your changes

This PR shuffles the subcommands around a little. This is in anticipation of some reworking for ctp connect.

This changeset builds on #388. #388 should be merged/reviewed prior to this PR.

I have:

How has this code been tested

./_output/bin/darwin_arm64/up ctp --help
Usage: up controlplane (ctp) <command>

Interact with control planes.

Flags:
  -h, --help                         Show context-sensitive help.
      --format="default"             Format for get/list commands. Can be: json, yaml, default
  -v, --version                      Print version and exit.
  -q, --quiet                        Suppress all output.
      --pretty                       Pretty print output.

      --domain=https://upbound.io    Root Upbound domain ($UP_DOMAIN).
      --profile=STRING               Profile used to execute command ($UP_PROFILE).
  -a, --account=STRING               Account used to execute command ($UP_ACCOUNT).
      --insecure-skip-tls-verify     [INSECURE] Skip verifying TLS certificates
                                     ($UP_INSECURE_SKIP_TLS_VERIFY).
  -d, --debug=INT                    [INSECURE] Run with debug logging. Repeat to increase verbosity.
                                     Output might contain confidential data like tokens ($UP_DEBUG).

Commands:
  controlplane (ctp) create           Create a managed control plane.
  controlplane (ctp) delete           Delete a control plane.
  controlplane (ctp) list             List control planes for the account.
  controlplane (ctp) get              Get a single control plane.
  controlplane (ctp) connector        Connect an App Cluster to a managed control plane.
  controlplane (ctp) configuration    Manage Configurations.
  controlplane (ctp) provider         Manage Providers.
  controlplane (ctp) pull-secret      Manage package pull secrets.
  controlplane (ctp) kubeconfig       Manage control plane kubeconfig data.
./_output/bin/darwin_arm64/up ctp connector --help
Usage: up controlplane (ctp) connector <command>

Connect an App Cluster to a managed control plane.

Flags:
  -h, --help                         Show context-sensitive help.
      --format="default"             Format for get/list commands. Can be: json, yaml, default
  -v, --version                      Print version and exit.
  -q, --quiet                        Suppress all output.
      --pretty                       Pretty print output.

      --domain=https://upbound.io    Root Upbound domain ($UP_DOMAIN).
      --profile=STRING               Profile used to execute command ($UP_PROFILE).
  -a, --account=STRING               Account used to execute command ($UP_ACCOUNT).
      --insecure-skip-tls-verify     [INSECURE] Skip verifying TLS certificates
                                     ($UP_INSECURE_SKIP_TLS_VERIFY).
  -d, --debug=INT                    [INSECURE] Run with debug logging. Repeat to increase verbosity.
                                     Output might contain confidential data like tokens ($UP_DEBUG).

Commands:
  controlplane (ctp) connector install    Install mcp-connector into an App Cluster.
AlainRoy commented 10 months ago

Wait, I looked that the PRs in the wrong order: this overlaps #388. I'm guessing this PR needs to be updated? Maybe you should ignore my approval there.

tnthornton commented 10 months ago

Wait, I looked that the PRs in the wrong order: this overlaps https://github.com/upbound/up/pull/388. I'm guessing this PR needs to be updated? Maybe you should ignore my approval there.

Ya, this is branched from #388 and builds on that guy. I called that out in the description too 👍 . I'm guessing this is the reason for this comment:

One question: The description for the PR sounds much more limited than what is actually here. Is the description wrong, or did you not intend to include everything in this PR? I'm okay with it all being included, just making sure it's what you expect.

?

AlainRoy commented 10 months ago

Wait, I looked that the PRs in the wrong order: this overlaps #388. I'm guessing this PR needs to be updated? Maybe you should ignore my approval there.

Ya, this is branched from #388 and builds on that guy. I called that out in the description too 👍 . I'm guessing this is the reason for this comment:

One question: The description for the PR sounds much more limited than what is actually here. Is the description wrong, or did you not intend to include everything in this PR? I'm okay with it all being included, just making sure it's what you expect.

?

Yeah, I just got confused since it's awaiting the merge, so there was more code. It all looks good though.