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

RHOAIENG-15772: tests(odh-nbc): write envtest kubeconfig to disk upon request #450

Merged

Conversation

jiridanek
Copy link
Member

@jiridanek jiridanek commented Nov 14, 2024

https://issues.redhat.com/browse/RHOAIENG-15772

Description

This helps with debugging because it's then possible to do e.g.

KUBECONFIG=/tmp/envtest.kubeconfig k9s

and investigate the cluster while the test is paused on a breakpoint.

How Has This Been Tested?

Works on my macOS laptop.

Merge criteria:

  • Any internal docs about the env variable needed?
  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@openshift-ci openshift-ci bot requested review from atheo89 and jstourac November 14, 2024 15:12
@jiridanek jiridanek force-pushed the jd_write_out_kubeconfig branch from 0033fac to 91ba6ad Compare November 14, 2024 15:15
@jstourac
Copy link
Member

@jiridanek thanks for this

/lgtm

Maybe you could put some note in the readme or somewhere? Just state that this property exists, what it is for and how it can be utilized. So that anyone can see easily? 🙂

@@ -129,6 +130,16 @@ var _ = BeforeSuite(func() {
})
Expect(err).NotTo(HaveOccurred())

if kubeconfigPath, found := os.LookupEnv("DEBUG_WRITE_KUBECONFIG"); found {
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe you could put some note in the readme or somewhere? Just state that this property exists, what it is for and how it can be utilized. So that anyone can see easily? 🙂

Yeah, good point. Does the env variable name make sense? And should there be a log message about creating/not creating the kubeconfig file?

Copy link
Member

Choose a reason for hiding this comment

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

No problem with the variable name from my side. Also, log message is a good idea - would be nice to print it including the path where this file will be written out.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added both

… request

This helps with debugging because it's then possible to do e.g.

```
KUBECONFIG=/tmp/envtest.kubeconfig k9s
```

and investigate the cluster while the test is paused on a breakpoint.
@jiridanek jiridanek force-pushed the jd_write_out_kubeconfig branch from 91ba6ad to b642e51 Compare November 19, 2024 09:46
@openshift-ci openshift-ci bot removed the lgtm label Nov 19, 2024
@jiridanek
Copy link
Member Author

golangci-lint went crazy after I rebased?

@jstourac
Copy link
Member

Okay, the linter failures will be handled by different issue (jira tracker exists already); the failed two unit tests is a known flakyness (also tracked in a jira).

Thank you for the update, Jiri. I put two comments, but LGTM otherwise!

Co-authored-by: Jan Stourac <jstourac@redhat.com>
@jiridanek
Copy link
Member Author

/approve

Copy link

openshift-ci bot commented Nov 19, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jiridanek

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

@jstourac
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Nov 19, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit ed444be into opendatahub-io:main Nov 19, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants