Skip to content
This repository was archived by the owner on Jul 11, 2023. It is now read-only.

cli: Added checks for without args osm metrics enable/disable command #5021

Merged
merged 8 commits into from
Sep 26, 2022

Conversation

mudit-01
Copy link
Contributor

@mudit-01 mudit-01 commented Aug 24, 2022

Added checks for without args osm metrics enable/disable command fixes #4448

Signed-off-by: mudit singh [email protected]

Description:

Testing done: yes
image

Affected area:

Functional Area
New Functionality [ ]
CI System [ ]
CLI Tool [ X]
Certificate Management [ ]
Control Plane [ ]
Demo [ ]
Documentation [ ]
Egress [ ]
Ingress [ ]
Install [ ]
Networking [ ]
Observability [ ]
Performance [ ]
SMI Policy [ ]
Security [ ]
Sidecar Injection [ ]
Tests [ ]
Upgrade [ ]
Other [ ]

Please answer the following questions with yes/no.

  1. Does this change contain code from or inspired by another project?

    • Did you notify the maintainers and provide attribution?
  2. Is this a breaking change?

  3. Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)?

Added check if no namespace is mentioned.

Signed-off-by: mudit singh <[email protected]>
Added check is no namespace is mentioned.

Signed-off-by: mudit singh <[email protected]>
Added test cases for enabling and disabling metrics without namespace.

Signed-off-by: mudit singh <[email protected]>
@mudit-01 mudit-01 marked this pull request as ready for review August 24, 2022 12:24
@mudit-01 mudit-01 changed the title Bug osmetrics cli: Added checks for without args osm metrics enable/disable command Aug 24, 2022
Copy link
Contributor

@jsturtevant jsturtevant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! left a few comments for small adjustments

Added required flag of namespace in cobra command

Signed-off-by: mudit singh <[email protected]>
Added test for checking mandatory namespace args

Signed-off-by: mudit singh <[email protected]>
@mudit-01 mudit-01 marked this pull request as draft September 13, 2022 16:21
Added comment to ignore lint error of MarkFlagRequired

Signed-off-by: mudit singh <[email protected]>
Added MarkFlagRequired to check for namespace for cobra command

Signed-off-by: mudit singh <[email protected]>
@mudit-01 mudit-01 marked this pull request as ready for review September 13, 2022 17:30
@mudit-01
Copy link
Contributor Author

Thanks!, @jsturtevant for the review

@mudit-01
Copy link
Contributor Author

@mudit-01
Copy link
Contributor Author

Thanks!, @keithmattix for the review

@mudit-01
Copy link
Contributor Author

@jaellio @shashankram @steeling @trstringer @nojnhuh can I get another review on this, please?

@jaellio jaellio merged commit 87fd86c into openservicemesh:main Sep 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"osm metrics enable/disable" are effectively no-ops without args
5 participants