Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix airshipctl config get-context maximum args #526

Closed
SirishaGopigiri opened this issue Apr 16, 2021 · 3 comments
Closed

Fix airshipctl config get-context maximum args #526

SirishaGopigiri opened this issue Apr 16, 2021 · 3 comments
Assignees
Labels
bug Something isn't working size s
Milestone

Comments

@SirishaGopigiri
Copy link
Contributor

Bug Description
The airshipctl config get-context command currently doesn't limit on the number of arguments that can be passed. So when we pass more that one parameter(example as show below) it returns all the contexts.

airshipctl config get-context ephemeral-cluster target-cluster

Steps To Reproduce
Sample airship config file

apiVersion: airshipit.org/v1alpha1
managementConfiguration:
  dummy_management_config:
    type: redfish
    insecure: true
    useproxy: false
    systemActionRetries: 30
    systemRebootDelay: 30
contexts:
  test-cluster:
    contextKubeconf: ephemeral-cluster_ephemeral
    manifest: dummy_manifest
    managementConfiguration: dummy_management_config
  ephemeral-cluster:
    contextKubeconf: ephemeral-cluster_ephemeral
    manifest: dummy_manifest
    managementConfiguration: dummy_management_config
  target-cluster:
    contextKubeconf: target-cluster_target
    manifest: dummy_manifest
    managementConfiguration: dummy_management_config
currentContext: ephemeral-cluster
kind: Config
manifests:
  dummy_manifest:
    phaseRepositoryName: primary
    repositories:
      primary:
        checkout:
          branch: master
          force: false
          remoteRef: ""
          tag: ""
        url: https://review.opendev.org/airship/airshipctl
    metadataPath: manifests/site/test-site/metadata.yaml
    targetPath: xyz

For the above config file if we issue airshipctl config get-context ephemeral-cluster target-cluster it returns all the contexts. I think it would be better if we accept only one argument/parameter.

Expected behavior
Current output

managementConfiguration: dummy_management_config
manifest: dummy_manifest

managementConfiguration: dummy_management_config
manifest: dummy_manifest

managementConfiguration: dummy_management_config
manifest: dummy_manifest

Expected output:

accepts at most 1 arg(s), received 2

Simple fix would be to add Args: cobra.MaximumNArgs(1), in the cobra command

Environment

  • airshipctl Version: master branch
  • Operating System: macOS
  • Go version: 1.15.6
@SirishaGopigiri SirishaGopigiri added bug Something isn't working triage Needs evaluation by project members labels Apr 16, 2021
@airshipbot
Copy link

airshipbot commented Apr 16, 2021

Related Change #786654

Subject: Fix airshipctl config get-context maximum args
Link: https://review.opendev.org/c/airship/airshipctl/+/786654
Status: MERGED
Owner: Sirisha Gopigiri ([email protected])

This change will close this issue when merged.

Approvals

Code-Review
+2 Roman Gorshunov
+1 Bijaya Sharma
+1 Sujeet Chaudhary
+2 Kostyantyn Kalynovskyi
Verified
+1 ATT Airship2.0 CI
+2 Zuul
Workflow
+1 Kostyantyn Kalynovskyi

Last Updated: 2021-04-28 23:18:59 CDT

@SirishaGopigiri
Copy link
Contributor Author

Please assign this to me and here is the PS related to this fix https://review.opendev.org/c/airship/airshipctl/+/786654

Thank you!

@eak13 eak13 removed the triage Needs evaluation by project members label Apr 20, 2021
@eak13 eak13 added this to the v2.1 milestone Apr 20, 2021
@SirishaGopigiri
Copy link
Contributor Author

Size is 'S', thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size s
Projects
None yet
Development

No branches or pull requests

4 participants