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

[olm-cmd] Add "version" flag to olm commands #3279

Merged

Conversation

varshaprasad96
Copy link
Member

@varshaprasad96 varshaprasad96 commented Jun 23, 2020

Description of the change:
This PR adds "--version" flag to olm sub-commands, thereby enabling
users to specify the version of olm to uninstall or look up for
while fetching the status of its resources.

Motivation for the change:
Allow users to override the version of OLM inferred from packageservers's CSV

Closes : #3095

Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

Only one major issue I see.

/ping @joelanford

cmd/operator-sdk/olm/install.go Outdated Show resolved Hide resolved
internal/olm/manager.go Outdated Show resolved Hide resolved
Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

Just some minor doc nits.

website/content/en/docs/olm-integration/user-guide.md Outdated Show resolved Hide resolved
cmd/operator-sdk/olm/status.go Outdated Show resolved Hide resolved
@@ -19,6 +19,7 @@ operator-sdk olm status [flags]
-h, --help help for status
--olm-namespace string namespace where OLM is installed (default "olm")
Copy link
Member

Choose a reason for hiding this comment

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

Follow-up: add --olm-namespace to uninstall

This PR adds "--version" flag to olm commands, thereby enabling
users to specify the version of olm to uninstall or look for
while fetching the status of its resources.

# Uninstall should fail with OLM not installed
commandoutput=$(operator-sdk olm uninstall 2>&1 || true)
echo $commandoutput | grep -F "Failed to uninstall OLM: no existing installation found"
echo $commandoutput | grep -F "Failed to uninstall OLM"
Copy link
Member Author

@varshaprasad96 varshaprasad96 Jun 24, 2020

Choose a reason for hiding this comment

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

Since we are logging the reason of failure in error too, checking just whether installation/fetching status failed or not (for CI to pass).

Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 24, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 24, 2020
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@varshaprasad96 varshaprasad96 merged commit 15bdeea into operator-framework:master Jun 24, 2020
@varshaprasad96 varshaprasad96 deleted the olm-command/test branch June 24, 2020 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

operator-sdk olm status is unable to install olm
4 participants