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

Fix broken deployments and exercises #116

Merged
merged 10 commits into from
Jul 3, 2023

Conversation

CloudoguSiebels
Copy link
Contributor

No description provided.

CloudoguSiebels and others added 6 commits June 23, 2023 13:23
apparently having recurse:true in combination with a
helm umbrella chart results in Argo CD not deploying the application.
even when using k3d which does not support that.
these were restructured. The new apt repository is "example-apps"
@schnatterer schnatterer force-pushed the feature/fix-broken-deployments-and-exercises branch from 047b77f to c45baf3 Compare June 26, 2023 13:54
Typo and ces-build-lib "out of sync" with other occurrences in the Playground.
@schnatterer
Copy link
Member

schnatterer commented Jun 27, 2023

As part of my review, I followed the READMEs of the exercises petclinic-helm and nginx-validation and found two things worth discussing.

Lowering the barriers

Both READMEs state the following:

The first step you have to take is to copy this repository under the namespace argocd in order for jenkins to pick it up.

I see two things to improve here.

First, Jenkins is not picking up the repo automatically, because it's not authorized.
To solutions come to mind:

  • The user could the CI-SERVER role to the user gitops (e.g. via localhost:9091/scm/repo/argocd/nginx-validation/settings/permissions) (which we could describe in the READMEs) or
  • we could authorize the user on the namespace level (localhost:9091/scm/namespace/argocd/settings/permissions) via the GitOps playground automatically. This would make the exercise simpler, but is not exactly least privilege for a production setting.

Which solution does sound better for you?

Secondly, we could mention SCMM's import functionality (/repos/create/import), which might be a lot easier than using the git CLI for a lot of people.

petclinic-helm OOM

After solving the exercise, the application ends up in CrashLoopBackoff due to OOM exception.

Screenshot from 2023-06-26 18-29-24

I was wondering why this does not happen to argocd/petclinic-helm:

memory: 750Mi
requests:
memory: 750Mi

Here, we increase the value (because it run two containers).
But obviously the 650MB we use by default here:
https://github.com/cloudogu/spring-boot-helm-chart/blob/0.3.0/values.yaml#L49-L51 no longer suffice for a simple spring boot app using the latest build pack builder version. I suggest we increase (both values) to 700MB, either in the Chart directly (we only use it for the playground) release a new version and update to it in the playground here:
SPRING_BOOT_HELM_CHART_COMMIT=0.3.0

Or we upgrade the values shared, just like in the example above
https://github.com/cloudogu/gitops-playground/blob/40ca6a85db2e2a76df671dc2ebb585a64590dd49/exercises/petclinic-helm/k8s/values-shared.yaml

We could also consider pinning the builder version to get a reproducible result. We do this in the argocd/petclinic-helm:

"-Dspring-boot.build-image.builder=paketobuildpacks/builder:0.3.229-base "

but not in the exercise:

mvn "spring-boot:build-image -DskipTests -Dcheckstyle.skip -Dspring-boot.build-image.imageName=${imageName}"

@CloudoguSiebels
Copy link
Contributor Author

First, Jenkins is not picking up the repo automatically, because it's not authorized.

We configure Jenkins to pick up all projects in that namespace. I think it's coherent to grant access to the entire namespace.

Myself, I moved the repository from one namespace to the other and did not run into issues. Except when redeploying the playground iteratively. Then, it is missing the exercise in the exercise repository.
I think mentioning the import feature is a good idea.

petclinic-helm OOM

I think we should fix this in the chart directly. There we explicitly set a value that probably never works anymore.

Additionally pinning the builder version is a good idea as well.

@schnatterer
Copy link
Member

Sounds good!

Moving the repo is a very nice find! The only downside is that in contexts where several users might want to take the exercise on the same playground instance the exercise is gone after moving.

So let's go for authorizing on namespace level. Let's start with CI_SEVER role, which is read-only. If Jenkins is supposed to write to a repo we'll authorize this separately. This way, we still have a kind of least privilege.

Otherwise, the user would have to manually add permissions to newly
created repositories when working through the exercise.
We don't want the user to need to worry about this.
Otherwise we couldn't build the petclinic anymore.
The petclinic has increased memory limits to make it start again.

By pinning the builder version, we ensure that increased memory
requirements don't stop the build/application from running.
@schnatterer schnatterer merged commit f73482e into main Jul 3, 2023
@schnatterer schnatterer deleted the feature/fix-broken-deployments-and-exercises branch July 3, 2023 11:21
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.

2 participants