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

e2e: adopt contrasttest framework #353

Merged
merged 5 commits into from
Apr 22, 2024
Merged

Conversation

malt3
Copy link
Contributor

@malt3 malt3 commented Apr 18, 2024

I tried to faithfully copy the small differences I saw between the static deployment files (emojivoto-sm-{in|e}gress) as part of generateEmojivoto.
@3u13r please confirm if my choices are correct.

@malt3 malt3 added the no changelog PRs not listed in the release notes label Apr 18, 2024
@malt3 malt3 force-pushed the e2e/adopt-contrasttest-framework branch from fad9f1c to bb4d1ba Compare April 18, 2024 14:28
@malt3 malt3 requested review from burgerdev and 3u13r April 18, 2024 14:39
@malt3 malt3 marked this pull request as ready for review April 18, 2024 14:39
@malt3 malt3 requested a review from katexochen as a code owner April 18, 2024 14:39
Copy link
Contributor

@burgerdev burgerdev left a comment

Choose a reason for hiding this comment

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

lgtm

e2e/internal/kuberesource/wrappers.go Outdated Show resolved Hide resolved
@malt3 malt3 force-pushed the e2e/adopt-contrasttest-framework branch from bb4d1ba to 54bf60b Compare April 18, 2024 15:01
3u13r
3u13r previously requested changes Apr 18, 2024
Copy link
Member

@3u13r 3u13r left a comment

Choose a reason for hiding this comment

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

I'm not sure if we want to test both the ingress and the egress separately. Currently, the "ingress" Test/Deployment also contains the egress configuration.
Either we have one (correctly named) test or we split the egress and ingress configuration.
If we choose to go with the second option, we need to configure the deployment similar to third commit of the PR: https://github.com/edgelesssys/contrast/pull/186/commits where the my "3u13r/web" image is used but the microservices use the ingress service mesh.

e2e/internal/kuberesource/mutators.go Outdated Show resolved Hide resolved
e2e/internal/kuberesource/sets.go Outdated Show resolved Hide resolved
e2e/internal/kuberesource/wrappers.go Outdated Show resolved Hide resolved
e2e/internal/kuberesource/parts.go Outdated Show resolved Hide resolved
e2e/internal/kuberesource/sets.go Outdated Show resolved Hide resolved
@malt3 malt3 force-pushed the e2e/adopt-contrasttest-framework branch from 54bf60b to 2966a56 Compare April 19, 2024 09:02
@malt3
Copy link
Contributor Author

malt3 commented Apr 19, 2024

I'm not sure if we want to test both the ingress and the egress separately. Currently, the "ingress" Test/Deployment also contains the egress configuration.

Thank you for clarifying. I opted to rename the configuration to reflect that we are testing ingress and egress together.

@malt3 malt3 requested review from 3u13r and katexochen April 19, 2024 09:11
@malt3 malt3 dismissed 3u13r’s stale review April 22, 2024 07:25

Change requests adressed

@malt3 malt3 merged commit 822dee4 into main Apr 22, 2024
8 checks passed
@malt3 malt3 deleted the e2e/adopt-contrasttest-framework branch April 22, 2024 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog PRs not listed in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants