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

Tests should use unique names when copying secrets into test namespace #3133

Closed
mgencur opened this issue Jun 5, 2023 · 0 comments · Fixed by #3134
Closed

Tests should use unique names when copying secrets into test namespace #3133

mgencur opened this issue Jun 5, 2023 · 0 comments · Fixed by #3134
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@mgencur
Copy link
Contributor

mgencur commented Jun 5, 2023

Describe the bug
Some tests copy secrets from the default namespace into the test namespace.
Example 1
Example 2

The secrets should be copied under unique names into the target namespace, this would allow for:

  • running multiple tests using the secrets in parallel
  • use a common/single namespace to run the tests (e.g. when testing with Istio in a pre-defined namespace)
  • proper teardown of resources (secrets) even if tests run in parallel

Currently, the secrets are copied into the test namespace and left there until the namespace is deleted. When running CreateSecretsAfterKafkaSource in the same namespace again the secret is already there and the test fails.

Expected behavior
Each test is using unique secrets which are removed when the test ends no matter if the namespace is removed.

To Reproduce

Knative release version

Additional context

@mgencur mgencur added the kind/bug Categorizes issue or PR as related to a bug. label Jun 5, 2023
knative-prow bot pushed a commit to knative/eventing that referenced this issue Jul 17, 2023
…7002)

Supports
knative-extensions/eventing-kafka-broker#3133
which is being implemented in
knative-extensions/eventing-kafka-broker#3134 and
requires these changes in knative eventing.

<!-- Please include the 'why' behind your changes if no issue exists -->

## Proposed Changes

<!-- Please categorize your changes:
- 🎁 Add new feature
- 🐛 Fix bug
- 🧹 Update or clean up current behavior
- 🗑️ Remove feature or internal logic
-->

-
-
-

### Pre-review Checklist

<!-- If these boxes are not checked, you will be asked to complete these
requirements or explain why they do not apply to your PR. -->

- [ ] **At least 80% unit test coverage**
- [ ] **E2E tests** for any new behavior
- [ ] **Docs PR** for any user-facing impact
- [ ] **Spec PR** for any new API feature
- [ ] **Conformance test** for any change to the spec

**Release Note**

<!--
📄 If this change has user-visible impact, write a release
note in the block
below. Include the string "action required" if additional action is
required of
users switching to the new release, for example in case of a breaking
change.

Write as if you are speaking to users, not other Knative contributors.
If this
change has no user-visible impact, no release note is needed.
-->

```release-note

```


**Docs**

<!--
📖 If this change has user-visible impact, link to an issue or PR in
https:/knative/docs.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant