Skip to content

always print name of resources created by must-gather#22561

Merged
deads2k merged 2 commits intoopenshift:masterfrom
sanchezl:oc_adm_must-gather_output
Apr 15, 2019
Merged

always print name of resources created by must-gather#22561
deads2k merged 2 commits intoopenshift:masterfrom
sanchezl:oc_adm_must-gather_output

Conversation

@sanchezl
Copy link
Copy Markdown
Contributor

namespace/openshift-must-gather-rfvmp created
clusterrolebinding.rbac.authorization.k8s.io/must-gather-6s6rb created
receiving incremental file list
created directory must-gather.local.312900838098712312
.
.
.
sent 24,843 bytes  received 117,399,750 bytes  2,472,096.69 bytes/sec
total size is 117,259,199  speedup is 1.00
clusterrolebinding.rbac.authorization.k8s.io/must-gather-6s6rb deleted
namespace/openshift-must-gather-rfvmp deleted

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 12, 2019
@sanchezl sanchezl mentioned this pull request Apr 12, 2019
8 tasks
if !o.Keep {
defer func() {
err = o.Client.CoreV1().Namespaces().Delete(ns.Name, nil)
if err = o.Client.CoreV1().Namespaces().Delete(ns.Name, nil); err == nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't pretend to scope to the if.

if !o.Keep {
defer func() {
err = o.Client.RbacV1().ClusterRoleBindings().Delete(clusterRoleBinding.Name, &metav1.DeleteOptions{})
if err = o.Client.RbacV1().ClusterRoleBindings().Delete(clusterRoleBinding.Name, &metav1.DeleteOptions{}); err == nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this would stomp the failing if from above. seems like you may want a channel?

@sanchezl sanchezl force-pushed the oc_adm_must-gather_output branch from 038f9db to ed1c99d Compare April 12, 2019 19:56
err2 := o.Client.CoreV1().Namespaces().Delete(ns.Name, nil)
if err2 != nil {
if err != nil {
err = fmt.Errorf("%v\n%v", err, err2)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah... don't do this. use a channel if we need a channel. For now, just suppress the error code and fmt.Printf() it out.

@sanchezl sanchezl force-pushed the oc_adm_must-gather_output branch from ed1c99d to 85475e8 Compare April 13, 2019 04:47
defer func() {
err = o.Client.CoreV1().Namespaces().Delete(ns.Name, nil)
if err := o.Client.CoreV1().Namespaces().Delete(ns.Name, nil); err != nil {
fmt.Printf("%v", err)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On monday, use Fprintf to direct to the error stream and put a newline on it, but this better than where we are today and in the hopes of landing for a beta build I'll merge.

@deads2k
Copy link
Copy Markdown
Contributor

deads2k commented Apr 13, 2019

/retest
/lgtm

lgetm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 13, 2019
@openshift-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, sanchezl

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

The pull request process is described here

Details 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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 13, 2019
@mfojtik
Copy link
Copy Markdown
Contributor

mfojtik commented Apr 13, 2019

/retest

1 similar comment
@deads2k
Copy link
Copy Markdown
Contributor

deads2k commented Apr 15, 2019

/retest

@deads2k
Copy link
Copy Markdown
Contributor

deads2k commented Apr 15, 2019

we need a debug tool to debug the failures this is flaking on. merging

@deads2k deads2k merged commit fb2cca9 into openshift:master Apr 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants