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

feat: add initial setup for portal and cx-iam #38

Merged
merged 4 commits into from
Nov 27, 2023

Conversation

evegufy
Copy link
Contributor

@evegufy evegufy commented Sep 8, 2023

Description

This PR adds the helm charts from portal and cx-iam (versions from 3.2/23.09 release) to the umbrella chart.
It doesn't contain tests in this sense and for a proper proper e2e testing functional tests as wells as more config would need to get added, still, for an initial integration in the umbrella helm chart this PR should be sufficient.

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

@carslen
Copy link
Contributor

carslen commented Sep 11, 2023

PR looks straight forward. Are these failing tests ok, @almadigabor ? I don't get the reason for the failure from error messages for both failed checks.

@almadigabor
Copy link
Contributor

I don't think the errors are related to the pull request. Kyverno fails as some of the dependencies have not set runAsNonRoot in the pod specification. Helm test fails with the edc dataplane pod if I get it correctly.

@evegufy
Copy link
Contributor Author

evegufy commented Sep 13, 2023

@carslen @almadigabor thanks for reviewing! Last weeks I successfully tested the install on my local Minikube.
IMO the issue with ct here, maybe upgrading the helm testing action would be worth a try or replacing it entirely with a simple helm install.

@fty4
Copy link
Member

fty4 commented Sep 25, 2023

I just tried to start this branch locally in my minikube and had following problems:

  • two pods will keep restarting (processes-worker and e2e-sharedidp)
  • some pods does not have the given Helm install name as prefix

IMO the issue with ct here, maybe upgrading the helm testing action would be worth a try or replacing it entirely with a simple helm install.

Regarting to your last comment, @evegufy: I do not think we should replace the ct with a helm install - this test framework is quiet common and will print a detailed log.

For example: in the last run of this pr you can see that the runner might be to small to run that heavy load:0/1 nodes are available: 1 Insufficient cpu. preemption: 0/1 nodes are available: 1 No preemption victims found for incoming pod.

We might need to use larger runner or try to limit the load.

@evegufy
Copy link
Contributor Author

evegufy commented Sep 25, 2023

I just tried to start this branch locally in my minikube and had following problems:

  • two pods will keep restarting (processes-worker and e2e-sharedidp)
  • some pods does not have the given Helm install name as prefix

IMO the issue with ct here, maybe upgrading the helm testing action would be worth a try or replacing it entirely with a simple helm install.

Regarting to your last comment, @evegufy: I do not think we should replace the ct with a helm install - this test framework is quiet common and will print a detailed log.

For example: in the last run of this pr you can see that the runner might be to small to run that heavy load:0/1 nodes are available: 1 Insufficient cpu. preemption: 0/1 nodes are available: 1 No preemption victims found for incoming pod.

We might need to use larger runner or try to limit the load.

@fty4 Thanks for testing!

Regarding 'processes-worker': that's cronJob, and it's normal that it restarts every couple of minutes
Regarding 'sharedidp': what's your local OS? I tested on Ubuntu. If you're using Mac with Apple Silicon, the reason for the restarts could be the mismatching cpu arch. The image is only available for amd64, in this case it can help to provide more cpu cores to your Minikube to enable proper emulation of the different cpu arch, but the performance on Mac will even then probably not be great and the pod will need a couple of restarts to start up.

Good hint regarding the insufficient cpu. We probably need to add a config for the KinD action as the default values aren't sufficient.

I'm still not quite convinced if ct is really suitable for testing such a chart:
When having enabled both keycloak instances (sharedidp and centralidp), I encountered for instance this error which I wasn't encountering on my local Minikube.
Also, in another context, I already ran into helm/chart-testing#332 but this could be not an issue if we upgrade helm testing action.
Still, I'm open to try some other things instead of removing ct.

@almadigabor almadigabor marked this pull request as ready for review November 27, 2023 09:59
@almadigabor almadigabor merged commit 183bf88 into main Nov 27, 2023
4 of 7 checks passed
@evegufy evegufy deleted the feat/initial-integration-portal_cx-iam branch February 14, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants