-
Notifications
You must be signed in to change notification settings - Fork 15
Appsody Operator must-gather #209
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @JusteenR. Just a few comments
stage: build must gather | ||
script: | ||
- make build-must-gather | ||
- name: Build must gather image and push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this stage is doing both build and push, do we still need Build must gather image
stage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was initially following the the operator build step but it isn't necessary for this so I removed the Build must gather image
stage.
CHANGELOG.md
Outdated
@@ -10,6 +10,10 @@ All notable changes to this project will be documented in this file. | |||
|
|||
## [Unreleased] | |||
|
|||
### Added | |||
|
|||
- Added must-gather scripts for troubleshooting. ([#148](https://github.com/appsody/appsody-operator/issues/148)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should point to PR(s). Please update the link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also seems like there is a conflict with the master branch in this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the link and fixed the conflicts.
doc/troubleshooting.md
Outdated
``` | ||
|
||
Note: `must-gather` flag is a new feature added to oc v4.x. If you are using an older version of oc, you can get a new version of the CLI from here. You can use oc v4.x against 3.11 cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use "OpenShift client CLI" instead of "oc".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link in "...CLI from here..." is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed oc to OpenShift client CLI and added the link.
@@ -0,0 +1 @@ | |||
# Appsody Operator must-gather |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add text to refer people to the troubleshooting doc
…teen Fixed CHANGELOG conflicts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @JusteenR
What this PR does / why we need it?:
Appsody Operator must-gather
is a tool built on top of OpenShift must-gather that expands its capabilities to gather information about the Appsody Operator.Does this PR introduce a user-facing change?
CHANGELOG.md
Which issue(s) this PR fixes:
Fixes #148