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.

Sign up for GitHub

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

Jump to bottom

OADP-1801: Include restic and velero binaries in must-gather image, and gather various pieces of version information. #994

Merged
merged 15 commits into from May 6, 2023
Merged

OADP-1801: Include restic and velero binaries in must-gather image, and gather various pieces of version information. #994

merged 15 commits into from May 6, 2023

Conversation

mrnold
Copy link
Contributor

@mrnold mrnold commented May 2, 2023 •
edited

Addresses OADP-1801, download and build restic and velero binaries and copy them to the must-gather image, and save various pieces of version information.

A backport should change the must-gather Dockerfile branch pointers for restic and velero to (for example) oadp-1.2.

@mrnold
Build restic and velero for must-gather image.
4035ff6 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@openshift-ci-robot
Copy link

openshift-ci-robot commented May 2, 2023 •
edited by openshift-ci bot

@mrnold: This pull request references OADP-1801 which is a valid jira issue.

In response to this:

Partial fix for OADP-1801, download and build restic and velero binaries and copy them to the must-gather image.

A backport should change the branch pointers to (for example) oadp-1.2.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 2, 2023
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 2, 2023
@openshift-ci
Copy link

openshift-ci bot commented May 2, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci-robot
Copy link

openshift-ci-robot commented May 2, 2023 •
edited by openshift-ci bot

@mrnold: This pull request references OADP-1801 which is a valid jira issue.

In response to this:

Partial fix for OADP-1801, download and build restic and velero binaries and copy them to the must-gather image.

A backport should change the branch pointers to (for example) oadp-1.2.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 2, 2023
@mrnold
Clean up build steps to avoid running out of space.
e4e8c51 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 2, 2023
@mrnold
Copy link
Contributor Author

mrnold commented May 2, 2023

The build failed with "No space left on device", so I added some cleanup steps as an avoidance.

mrnold added 2 commits May 3, 2023 13:58
@mrnold
Record versions of OADP and volsync operators.
05ba965 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@mrnold
Respect skip_tls option in gather_versions.
2dbb772 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@openshift-ci-robot
Copy link

openshift-ci-robot commented May 3, 2023 •
edited by openshift-ci bot

@mrnold: This pull request references OADP-1801 which is a valid jira issue.

In response to this:

Partial fix for OADP-1801, download and build restic and velero binaries and copy them to the must-gather image, and save version information for OADP/volsync operators.

A backport should change the must-gather Dockerfile branch pointers for restic and velero to (for example) oadp-1.2.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@mrnold
Move oadp-debug-info to cluster subdirectory.
5aded03 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@kaovilai
Copy link
Member

kaovilai commented May 3, 2023

Since we're adding version collections, we should add velero deployments too? that's one of the parameters that can break which is more than one velero versions installed.

So oc get deployment -A | grep velero
and oc exec /velero version

mrnold added 2 commits May 3, 2023 17:32
@mrnold
Get restic version in must-gather.
871a0c0 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@mrnold
Get velero client and deployment versions.
525d505 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@mrnold
Copy link
Contributor Author

mrnold commented May 3, 2023

Since we're adding version collections, we should add velero deployments too? that's one of the parameters that can break which is more than one velero versions installed.

So oc get deployment -A | grep velero and oc exec /velero version

I added this and the restic client version too, do you specifically want this to "oc exec" to a particular pod to get the velero version from there? Or is the one I have built-in to the must-gather image good enough?

mrnold added 2 commits May 3, 2023 21:24
@mrnold
Get storage classes (short) and DPA details.
91f63d9 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@mrnold
Add debug script directory and a small example.
d330687 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@mrnold mrnold mentioned this pull request May 4, 2023
Merged
@kaovilai
Copy link
Member

kaovilai commented May 4, 2023

is the one I have built-in to the must-gather image good enough?

That is probably good enough.. we just want to know the server version. I doubt that breaks.. could fall back to oc exec on error tho.

@mrnold
Create directory up front, avoid whole DPA CR.
ddf28b9 
DPAs are already logged by gather_crs.

Signed-off-by: Matthew Arnold <marnold@redhat.com>
@mrnold mrnold marked this pull request as ready for review May 4, 2023 18:07
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 4, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented May 4, 2023 •
edited by openshift-ci bot

@mrnold: This pull request references OADP-1801 which is a valid jira issue.

In response to this:

Addresses OADP-1801, download and build restic and velero binaries and copy them to the must-gather image, and save various pieces of version information.

A backport should change the must-gather Dockerfile branch pointers for restic and velero to (for example) oadp-1.2.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

weshayutin
weshayutin reviewed May 5, 2023
View reviewed changes
must-gather/collection-scripts/gather_versions Outdated
if [ -z "${storageclasses}" -o "${storageclasses}" == " " ]; then
log_command "StorageClass" "echo No StorageClasses found in cluster"
else
log_command "StorageClasses" "oc get storageclasses -o yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

great!

weshayutin
weshayutin reviewed May 5, 2023
View reviewed changes
must-gather/collection-scripts/gather_versions Outdated
else
while read dpa namespace; do
log_command "DataProtectionApplication ${namespace}/${dpa}" "oc get dpa -n ${namespace} ${dpa} -o yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

woot!

@mrnold mrnold changed the title OADP-1801: Include restic and velero binaries in must-gather image. OADP-1801: Include restic and velero binaries in must-gather image, and gather various pieces of version information. May 5, 2023
mrnold added 2 commits May 5, 2023 15:04
@mrnold
Remove default SC, add CSI VSC gathering.
96e206d 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@mrnold
Add defaults for debug file and skip_tls.
1cf93ca 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 5, 2023
kaovilai
kaovilai reviewed May 5, 2023
View reviewed changes
must-gather/collection-scripts/debug/tail_failed_pods Outdated
@@ -0,0 +1,8 @@
#!/bin/bash

for pod in $(oc get pods -n openshift-adp -o jsonpath='{.items[?(.status.containerStatuses[0].lastState.terminated.reason=="Error")].metadata.name}'); do
Copy link
Member

Choose a reason for hiding this comment

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

Do we need insecure_tls here?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for pod in $(oc get pods -n openshift-adp -o jsonpath='{.items[?(.status.containerStatuses[0].lastState.terminated.reason=="Error")].metadata.name}'); do
for pod in $(oc get pods -n openshift-adp -o jsonpath='{.items[?(.status.containerStatuses[0].lastState.terminated.reason=="Error")].metadata.name}' --insecure-skip-tls-verify=${skip_tls}); do

kaovilai
kaovilai reviewed May 5, 2023
View reviewed changes
must-gather/collection-scripts/gather_versions Outdated
if [ -z "${storageclasses}" -o "${storageclasses}" == " " ]; then
log_command "StorageClass" "echo No StorageClasses found in cluster"
else
log_command "StorageClasses" "oc get storageclasses -o yaml"
Copy link
Member

Choose a reason for hiding this comment

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

insecure_tls?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
log_command "StorageClasses" "oc get storageclasses -o yaml"
log_command "StorageClasses" "oc get storageclasses -o yaml --insecure-skip-tls-verify=${skip_tls}"

kaovilai
kaovilai reviewed May 5, 2023
View reviewed changes
must-gather/collection-scripts/gather_versions Outdated
log_command "DataProtectionApplication CRs" "echo No DPAs found in cluster"
else
while read dpa namespace; do
log_command "DataProtectionApplication ${namespace}/${dpa}" "oc get dpa -n ${namespace} ${dpa} -o yaml"
Copy link
Member

Choose a reason for hiding this comment

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

insecure tls?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
log_command "DataProtectionApplication ${namespace}/${dpa}" "oc get dpa -n ${namespace} ${dpa} -o yaml"
log_command "DataProtectionApplication ${namespace}/${dpa}" "oc get dpa -n ${namespace} ${dpa} -o yaml --insecure-skip-tls-verify=${skip_tls}"

@kaovilai
Copy link
Member

kaovilai commented May 5, 2023

/hold
unhold when addressed :)

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 5, 2023
@mrnold
Add missing --insecure-skip-tls-verify arguments.
5195fed 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 5, 2023
kaovilai
kaovilai reviewed May 5, 2023
View reviewed changes
must-gather/collection-scripts/debug/tail_failed_pods Outdated
for pod in $(oc get pods --insecure-skip-tls-verify=${skip_tls} -n openshift-adp -o jsonpath='{.items[?(.status.containerStatuses[0].lastState.terminated.reason=="Error")].metadata.name}'); do
echo "***"
echo "* Last logs from failed pod openshift-adp/${pod}:"
oc logs -n openshift-adp $pod --tail=10
Copy link
Member

Choose a reason for hiding this comment

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

Do you think we need insecure here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At some point probably all of this must-gather should be reviewed for this, there are a bunch of places that don't look at skip_tls at all.

@mrnold
One more missing --insecure-skip-tls-verify.
7b52ea0 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
kaovilai
kaovilai approved these changes May 5, 2023
View reviewed changes
Copy link
Member

@kaovilai kaovilai left a comment

Choose a reason for hiding this comment

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

/unhold

kaovilai
kaovilai approved these changes May 5, 2023
View reviewed changes
Copy link
Member

@kaovilai kaovilai left a comment

Choose a reason for hiding this comment

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

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 5, 2023
@kaovilai
Copy link
Member

kaovilai commented May 5, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 5, 2023
@openshift-ci
Copy link

openshift-ci bot commented May 5, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai, mrnold, weshayutin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

1 similar comment
@openshift-ci
Copy link

openshift-ci bot commented May 5, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai, mrnold, weshayutin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mrnold
Copy link
Contributor Author

mrnold commented May 5, 2023

/test 4.11-operator-e2e-gcp

@openshift-ci
Copy link

openshift-ci bot commented May 6, 2023

@mrnold: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit c007cd9 into openshift:master May 6, 2023
12 checks passed
@mrnold
Copy link
Contributor Author

mrnold commented May 6, 2023

/cherry-pick oadp-1.2

@openshift-cherrypick-robot
Copy link
Contributor

@mrnold: new pull request created: #1004

In response to this:

/cherry-pick oadp-1.2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request May 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weshayutin weshayutin weshayutin approved these changes

@kaovilai kaovilai kaovilai approved these changes

@savitharaghunathan savitharaghunathan Awaiting requested review from savitharaghunathan

Assignees

@weshayutin weshayutin

@kaovilai kaovilai

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@mrnold @openshift-ci-robot @kaovilai @openshift-cherrypick-robot @weshayutin @openshift-merge-robot