-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add e2e tests for our VSHN services #267
Conversation
d3a107f
to
d2fc989
Compare
d2fc989
to
8dfb658
Compare
347a53b
to
8dfb658
Compare
8dfb658
to
d5e529a
Compare
2f2adbe
to
07b4bff
Compare
07b4bff
to
963b5da
Compare
4a9efd9
to
4416f34
Compare
963b5da
to
95de839
Compare
5768d2d
to
d278800
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be a kuttl test with wrong identations.
d278800
to
a098a24
Compare
bfda383
to
d216fce
Compare
c351a0d
to
c10989a
Compare
Signed-off-by: Nicolas Bigler <[email protected]>
c10989a
to
822ed98
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some things to adjust.
@@ -424,6 +424,12 @@ local sgCluster = { | |||
}, | |||
postgres: { | |||
version: '', | |||
extensions: [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this, there's already a fix for this in the composition functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleted that section
tests/e2e/kuttl-test.yaml
Outdated
kind: TestSuite | ||
testDirs: | ||
- ./test/e2e/ | ||
namespace: widera-testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably want a more generic name here.
Like vshn-appcat-e2e-tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated that section
tests/e2e/keycloak/01-connect.yaml
Outdated
restartPolicy: Never | ||
containers: | ||
- name: connect | ||
image: docker.io/appuio/oc:v4.14 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This image does not work on Macs with Apple silicon: no match for platform in manifest: not found
. Not sure why though, as other x86 images usually work with no problem.
However, as you only need curl, maybe some other image would be better suited here? Maybe some small alpine image with curl installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree here. I also use a mac and a lighter image is welcomed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switched to another image also mentioned issue with that image on our #appuio-cloud RC channel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small things, rest is good.
@@ -6,7 +6,7 @@ parameters: | |||
version: 17.7.1 | |||
mariadb: | |||
source: https://charts.bitnami.com/bitnami | |||
version: 10.1.3 | |||
version: 11.6.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we update the chart version in an e2e PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because e2e tests found an issue in old charts :D
tests/e2e/keycloak/01-connect.yaml
Outdated
restartPolicy: Never | ||
containers: | ||
- name: connect | ||
image: docker.io/appuio/oc:v4.14 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree here. I also use a mac and a lighter image is welcomed.
aa6e4a9
to
b4edd76
Compare
b4edd76
to
d47cce7
Compare
@Kidswiss @zugao I just added new directory and file .githooks/pre-commit - it's small and simple python script to prevent user from commiting code when keycloak docker registry credentials are present in code, it's copied into .git/hooks directory whenever someone start compilation of golden files. Unfortunately this is the best protection we can apply because git doesn't like setting such script automatically |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Checklist
changelog.
The PR has a meaningful description that sums up the change. It will be
linked in the changelog.
bug
,enhancement
,documentation
,change
,breaking
,dependency
as they show up in the changelog.