build: track generated CSV, and validate #8862

BlaineEXE · 2021-09-28T22:03:48Z

Start tracking the generated CSV, and validate that they don't change in
builds.

Signed-off-by: Blaine Gardner blaine.gardner@redhat.com

Description of your changes:

Which issue is resolved by this Pull Request:
Resolves #

Checklist:

images/ceph/Makefile

BlaineEXE · 2021-09-29T17:36:22Z

Makefile

@@ -115,7 +115,7 @@ do.build.platform.%:

 do.build.parallel: $(foreach p,$(PLATFORMS), do.build.platform.$(p))

-build: csv-clean build.common ## Only build for linux platform


csv-clean here was causing changes to tracked CSVs on build.

BlaineEXE · 2021-09-29T20:50:40Z

Makefile

-csv-clean: ## Remove existing OLM files.
-	@$(MAKE) -C images/ceph csv-clean


csv-clean in general was causing changes to tracked CSVs on build, so I just remove it.

BlaineEXE · 2021-09-29T20:51:00Z

Makefile

-	@$(MAKE) -C images/ceph csv-clean
-
+GEN_CRD_TEMP := /tmp/rook-ceph-gen-crds
+BUILD_CRDS_INTO_DIR ?= $(GEN_CRD_TEMP) # unless overridden, build CRDs into the temp dir


So the intermediate steps don't interfere with tracked CSVs.

BlaineEXE · 2021-09-29T20:52:14Z

images/ceph/Makefile

-	@mkdir -p $(CSV_TEMPLATE_DIR)
-	@cp -a ../../cluster $(CSV_TEMPLATE_DIR)/cluster
-	@set -eE;\
+	@ echo yq=$(YQ) operator-sdk=$(OPERATOR_SDK)
+	mkdir -p $(CSV_TEMPLATE_DIR)
+	cp -a ../../cluster $(CSV_TEMPLATE_DIR)/cluster
+	set -eE;\


Removing @ symbols here and elsewhere was useful in debugging build issues in the CI for this PR and will be useful in the future as well, so I keep them.

How is it useful in debuging? Is it printing the commands?

Yes, exactly

Start tracking generated CSV templates, and validate that they don't change in builds. Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>

mergify · 2021-09-30T19:54:17Z

This pull request has merge conflicts that must be resolved before it can be merged. @BlaineEXE please rebase it. https://rook.io/docs/rook/latest/development-flow.html#updating-your-fork

leseb

Why do we have to track all the RBACs too? This looks like a lot of duplication from common.yaml, isn't it?

leseb · 2021-10-01T08:38:19Z

cluster/olm/ceph/deploy/operator.yaml

@@ -0,0 +1,151 @@
+# OLM: BEGIN OPERATOR DEPLOYMENT


Why is the operator's configmap gone?

Maybe the OLM generation is broken? I haven't touched any of that code. If it's not here, then I think it hasn't been getting generated for quite some time.

leseb · 2021-10-01T08:40:25Z

cluster/olm/ceph/generate-rook-csv.sh

-    "${YQ_CMD_WRITE[@]}" "$CSV_FILE_NAME" metadata.annotations.externalClusterScript "$(base64 <$CEPH_EXTERNAL_SCRIPT_FILE)"
+
+    if [[ "${GENERATE_ROOK_CSV_FOR_TRACKING_ONLY}" != "true" ]]; then
+        # base64 has different behavior on linux vs macos


what's the difference?

MacOS's base64 can only produce a hash on a single line. On Linux, it is multi-line by default.

leseb · 2021-10-01T08:41:59Z

images/ceph/Makefile

-	@mkdir -p $(CSV_TEMPLATE_DIR)
-	@cp -a ../../cluster $(CSV_TEMPLATE_DIR)/cluster
-	@set -eE;\
+	@ echo yq=$(YQ) operator-sdk=$(OPERATOR_SDK)
+	mkdir -p $(CSV_TEMPLATE_DIR)
+	cp -a ../../cluster $(CSV_TEMPLATE_DIR)/cluster
+	set -eE;\


How is it useful in debuging? Is it printing the commands?

BlaineEXE · 2021-10-07T15:52:16Z

Why do we have to track all the RBACs too? This looks like a lot of duplication from common.yaml, isn't it?

The RBAC used for CSV is not the same as RBAC from common.yaml. It is a subset. Unless we start providing all the RBAC from common.yaml, I think we should still version them here so we can tell when something specifically related to CSV changes.

leseb · 2021-10-08T13:06:33Z

Why do we have to track all the RBACs too? This looks like a lot of duplication from common.yaml, isn't it?

The RBAC used for CSV is not the same as RBAC from common.yaml. It is a subset. Unless we start providing all the RBAC from common.yaml, I think we should still version them here so we can tell when something specifically related to CSV changes.

They should all be the same, if not it could be a bug...

github-actions · 2021-11-07T20:01:57Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

BlaineEXE · 2021-11-10T23:15:04Z

@leseb

I was wanting to revisit this for RBAC generation from the Helm chart (which will/could affect CSV generation). I think it may be best to track the CSV's RBAC for now, then start generating CSV RBAC using a Python script like https://github.com/rook/rook/blob/master/build/rbac/keep-rbac-yaml.py.

If your previous comment (#8862 (comment)) is true, then we should be including all the RBAC in the CSV, and we can stop using # OLM: {BEGIN,END} comments for the generation. This will show us what RBAC is being added compared to the existing set, and we will have a diff to check and see if anything needs to be pruned.

After that, we can decide if we should stop versioning the CSV RBAC or if we should keep it to verify changes to generated RBAC.

Unless you have a better idea for how we can keep track of what changes with generated CSV stuff?

leseb · 2021-11-26T16:34:34Z

@leseb

I was wanting to revisit this for RBAC generation from the Helm chart (which will/could affect CSV generation). I think it may be best to track the CSV's RBAC for now, then start generating CSV RBAC using a Python script like https://github.com/rook/rook/blob/master/build/rbac/keep-rbac-yaml.py.

If your previous comment (#8862 (comment)) is true, then we should be including all the RBAC in the CSV, and we can stop using # OLM: {BEGIN,END} comments for the generation. This will show us what RBAC is being added compared to the existing set, and we will have a diff to check and see if anything needs to be pruned.

After that, we can decide if we should stop versioning the CSV RBAC or if we should keep it to verify changes to generated RBAC.

Unless you have a better idea for how we can keep track of what changes with generated CSV stuff?

No, I think tracking the changes as part of git is good.

BlaineEXE · 2021-11-30T15:43:56Z

Closing in favor of #9200

BlaineEXE requested a review from leseb September 28, 2021 22:03

BlaineEXE force-pushed the generate-and-track-csv branch 13 times, most recently from d51c8b6 to 85c5e0e Compare September 29, 2021 16:05

BlaineEXE marked this pull request as ready for review September 29, 2021 16:08

BlaineEXE requested a review from travisn September 29, 2021 16:08

BlaineEXE force-pushed the generate-and-track-csv branch from 85c5e0e to 1ab776e Compare September 29, 2021 17:34

BlaineEXE commented Sep 29, 2021

View reviewed changes

images/ceph/Makefile Outdated Show resolved Hide resolved

BlaineEXE commented Sep 29, 2021

View reviewed changes

BlaineEXE force-pushed the generate-and-track-csv branch 3 times, most recently from bd2e09b to c81f89d Compare September 29, 2021 20:43

BlaineEXE commented Sep 29, 2021

View reviewed changes

BlaineEXE force-pushed the generate-and-track-csv branch 5 times, most recently from cab8f51 to 6ee770b Compare September 29, 2021 23:16

build: generate CSV templates, and validate in CI

c961a1f

Start tracking generated CSV templates, and validate that they don't change in builds. Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>

BlaineEXE force-pushed the generate-and-track-csv branch from 6ee770b to c961a1f Compare September 30, 2021 14:44

leseb reviewed Oct 1, 2021

View reviewed changes

travisn mentioned this pull request Oct 6, 2021

csi: No longer install the VolumeReplication CRDs from Rook #8845

Merged

10 tasks

github-actions bot added the stale Labeled by the stale bot label Nov 7, 2021

github-actions bot removed the stale Labeled by the stale bot label Nov 11, 2021

BlaineEXE closed this Nov 30, 2021

BlaineEXE deleted the generate-and-track-csv branch March 1, 2023 22:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: track generated CSV, and validate #8862

build: track generated CSV, and validate #8862

BlaineEXE commented Sep 28, 2021

BlaineEXE Sep 29, 2021

BlaineEXE Sep 29, 2021

BlaineEXE Sep 29, 2021

BlaineEXE Sep 29, 2021

leseb Oct 1, 2021

BlaineEXE Oct 7, 2021

mergify bot commented Sep 30, 2021

leseb left a comment

leseb Oct 1, 2021

BlaineEXE Oct 7, 2021

leseb Oct 1, 2021

BlaineEXE Oct 7, 2021

leseb Oct 1, 2021

BlaineEXE commented Oct 7, 2021

leseb commented Oct 8, 2021

github-actions bot commented Nov 7, 2021

BlaineEXE commented Nov 10, 2021

leseb commented Nov 26, 2021

BlaineEXE commented Nov 30, 2021

		@@ -115,7 +115,7 @@ do.build.platform.%:

		do.build.parallel: $(foreach p,$(PLATFORMS), do.build.platform.$(p))

		build: csv-clean build.common ## Only build for linux platform

		csv-clean: ## Remove existing OLM files.
		@$(MAKE) -C images/ceph csv-clean

build: track generated CSV, and validate #8862

build: track generated CSV, and validate #8862

Conversation

BlaineEXE commented Sep 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Sep 30, 2021

leseb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BlaineEXE commented Oct 7, 2021

leseb commented Oct 8, 2021

github-actions bot commented Nov 7, 2021

BlaineEXE commented Nov 10, 2021

leseb commented Nov 26, 2021

BlaineEXE commented Nov 30, 2021