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

Add SKR binding test #1184

Merged
merged 12 commits into from
Sep 25, 2024
Merged

Add SKR binding test #1184

merged 12 commits into from
Sep 25, 2024

Conversation

MarekMichali
Copy link
Contributor

@MarekMichali MarekMichali commented Sep 25, 2024 •
edited
Loading

Description

Add a test with the following steps:

  • Provision SKR
  • Create a binding and save kubeconfig
  • Init k8s client with the returned kubeconfig
  • Try to fetch a secret
  • Deprovision SKR

Related issue(s)

See #284

@kyma-bot kyma-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 25, 2024
@kyma-bot
Copy link
Contributor

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

@kyma-bot kyma-bot added the cla: yes Indicates the PR's author has signed the CLA. label Sep 25, 2024
@github-actions GitHub Actions
Copy link

Add one of following labels

- kind/feature -> Use it when you want to submit a new feature

- kind/enhancement -> Use it when you modify or improve an existing feature

- kind/bug -> Use it when you fix a bug

@kyma-bot kyma-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 25, 2024
@ralikio ralikio mentioned this pull request Sep 25, 2024
Closed
13 tasks
@kyma-bot kyma-bot added area/documentation Issues or PRs related to documentation size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 25, 2024
@MarekMichali
Copy link
Contributor Author

/kind enhancement

@kyma-bot kyma-bot added the kind/enhancement Categorizes issue or PR as related to modifying or improving an existing feature label Sep 25, 2024
@MarekMichali MarekMichali marked this pull request as ready for review September 25, 2024 12:34
@MarekMichali MarekMichali requested review from a team as code owners September 25, 2024 12:34
@kyma-bot kyma-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 25, 2024
@kyma-bot kyma-bot added the lgtm Looks good to me! label Sep 25, 2024
@ralikio ralikio added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 25, 2024
ralikio
ralikio reviewed Sep 25, 2024
View reviewed changes
testing/e2e/skr/skr-binding-test/index.js
});

it('Initiate K8s client with kubeconfig from binding', async function() {
await initializeK8sClient({kubeconfig: kubeconfigFromBinding.credentials});
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add an assertion that where you cannot access cluster before initialization is done? I am afraid that it might swallow errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change it to not provision and init in the first step.

@MarekMichali
Copy link
Contributor Author

/hold

IwonaLanger
IwonaLanger reviewed Sep 25, 2024
View reviewed changes
docs/contributor/05-10-e2e_tests.md Outdated Show resolved Hide resolved
docs/contributor/05-10-e2e_tests.md
@@ -126,6 +126,31 @@ The test executes the following steps:
make skr-networking-test
```

## Binding Tests
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder whether it should be service binding here and in line 135? If just binding is sufficient, please ignore this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will leave it as it is. It's documentation for the tests, so I think that's sufficient.

@kyma-bot kyma-bot removed the lgtm Looks good to me! label Sep 25, 2024
IwonaLanger
IwonaLanger previously approved these changes Sep 25, 2024
View reviewed changes
@kyma-bot kyma-bot added the lgtm Looks good to me! label Sep 25, 2024
@kyma-bot kyma-bot removed the lgtm Looks good to me! label Sep 25, 2024
@kyma-bot kyma-bot added the lgtm Looks good to me! label Sep 25, 2024
@kyma-bot kyma-bot removed the lgtm Looks good to me! label Sep 25, 2024
@MarekMichali
Copy link
Contributor Author

/unhold

@kyma-bot kyma-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 25, 2024
IwonaLanger
IwonaLanger previously approved these changes Sep 25, 2024
View reviewed changes
@kyma-bot kyma-bot added the lgtm Looks good to me! label Sep 25, 2024
@kyma-bot kyma-bot removed the lgtm Looks good to me! label Sep 25, 2024
@kyma-bot kyma-bot added the lgtm Looks good to me! label Sep 25, 2024
@kyma-bot kyma-bot merged commit 2859e9b into kyma-project:main Sep 25, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IwonaLanger IwonaLanger IwonaLanger approved these changes

@ralikio ralikio ralikio approved these changes

@jaroslaw-pieszka jaroslaw-pieszka jaroslaw-pieszka left review comments

Assignees

@jaroslaw-pieszka jaroslaw-pieszka

@ralikio ralikio

@IwonaLanger IwonaLanger

Labels
area/documentation Issues or PRs related to documentation cla: yes Indicates the PR's author has signed the CLA. kind/enhancement Categorizes issue or PR as related to modifying or improving an existing feature lgtm Looks good to me! size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MarekMichali @kyma-bot @jaroslaw-pieszka @ralikio @IwonaLanger