🌱 Verify plantuml image generation in CI

[typeid]

What this PR does / why we need it:
This PR adds generation of the plantuml images to CI through a new job, which runs on PRs. The job fails in case the generated images differ from the contained images in PRs.

This solution is not ideal:

1. I'm reinstalling git in CI, as I couldn't find an image containing plantuml and git, and am not sure if we should be building it ourselves.
2. The job refers to variables that are set in the Makefile, if e.g. the PLANTUML_VER changes, we might forget about this job.

Which issue(s) this PR fixes:
Fixes #9331

[k8s-ci-robot]

Hi @typeid. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[typeid]

I see the job doesn't run in the PR CI yet, is there a way to test it? I have only tried it locally by using act.

[killianmuldoon]

WDYT about having this be a make target - verify-diagrams which could then run in the verify job we already gave in a GitHub action. This means you would already have the code pulled and could use the same tools that are available.

The verify-gen make target is a good comparison.

[typeid]

Thanks for the quick review @killianmuldoon.

I think that does make sense, I did not see we already had all kind of verify jobs running in prow prechecks, I was too distracted by the github workflows.

At quick glance, it doesn't seem like the image we're using for the verify job contains plantuml. This would mean we either need it installed in that image, or we run generate-diagrams directly. I'm not sure the latter works (docker in docker?), but I will try to find out.

[killianmuldoon]

I'm not sure the latter works (docker in docker?), but I will try to find out.

Ideally we'd simply be able to run verify-diagrams as part of the verify job. That should ensure we have plantuml installed and then run the build. You're right about it probably not working with the docker image though - we'll need to find some solution for that. Do we have Java available in the image we use for the verify jobs?

[killianmuldoon]

/ok-to-test

[typeid]

I wasn't able to find java installed in the gcr.io/k8s-staging-test-infra/kubekins-e2e:v20230727-ea685f8747-1.28 image either.

Installing plantuml within the verification image via dnf results in a very old version (PlantUML version 1.2020.02). This old version also produces different images than the version we use (1.2023).

I see a few options here if we want to use the prow job:

• if we can update the k8s-staging-test-infra image to contain plantuml/java, we'd have to always keep the plantuml versions in sync with that image's version.
• if we don't want to update the image, we could still install java and then download the plantuml jar file on every CI run. This would offer more flexibility but bloats the job a lot, and definitely isn't best practice.

For either of these options, we also wouldn't be able to re-use the job calling docker run.

Thanks for adding the ok-to-test, it seems like the git diff failed because it was called in a wrong repository. I'll see if I can fix that in the meantime. :)

[killianmuldoon]

/area ci

[typeid]

/retest

[sbueringer]

Can we simply run the generate-diagrams target which runs plantuml via Docker? (the verify job already has docker-in-docker enabled)

(+ the usual verify around it)

[typeid]

@sbueringer Thanks for the hint, I did not expect that to work. The latest push should do the trick, sorry for the experimenting!

I also noticed that quite a few verify-* jobs aren't run in the verify make target, notably:

• verify-security
• verify-govulncheck
• verify-container-images

See:

ALL_VERIFY_CHECKS = licenses boilerplate shellcheck tiltfile modules gen conversions doctoc capi-book-summary diagrams

.PHONY: verify
verify: $(addprefix verify-,$(ALL_VERIFY_CHECKS)) lint-dockerfiles ## Run all verify-* targets

Is this intended? I believe they're not run in CI at the moment.

[killianmuldoon]

I also noticed that quite a few verify-* jobs aren't run in the verify make target

This is intended - these scans are currently not run on PRs and are instead run as part of the weekly security scan: https://github.com/kubernetes-sigs/cluster-api/blob/main/.github/workflows/weekly-security-scan.yaml

[killianmuldoon]

Looks great and seems to be working - can you try to make it fail - by changing the plantuml spec maybe - so we can see what the output of this job failing is like?

[killianmuldoon]

Error looks good:

make generate-diagrams-proposals
make[1]: Entering directory '/home/prow/go/src/sigs.k8s.io/cluster-api'
docker run -v /home/prow/go/src/sigs.k8s.io/cluster-api/docs:/docs  plantuml/plantuml:1.2023 /docs/proposals/**/*.plantuml
make[1]: Leaving directory '/home/prow/go/src/sigs.k8s.io/cluster-api'
diff --git a/docs/proposals/images/machine-health-check/mhc.png b/docs/proposals/images/machine-health-check/mhc.png
index 395a7bba2..a50e002f6 100644
Binary files a/docs/proposals/images/machine-health-check/mhc.png and b/docs/proposals/images/machine-health-check/mhc.png differ
generated diagrams are out of date, run make generate-diagrams
make: *** [Makefile:679: verify-diagrams] Error 1

If you revert that commit I think this one is good to go!

[killianmuldoon]

Thanks - great work!
/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 54ea2f9c84b2f51732cd451a855c2a7afb97d0f2

[killianmuldoon]

/retitle 🌱 Verify plantuml image generation in CI

[sbueringer]

Thank you very much!!

Pretty happy that we don't have to deal with Java 😂

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [sbueringer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment