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

ci: add ability to have replaces,skips in CSV #147

Merged
merged 1 commit into from
Apr 20, 2022

Conversation

Rakshith-R
Copy link
Member

Each CSV has a replaces parameter that indicates
which Operator it replaces. This builds a graph of
CSVs that can be queried by OLM, and updates can be
shared between channels. Channels can be thought of
as entry points into the graph of updates:
REPLACES ?= ""

Creating the New CatalogSource requires publishing CSVs
that replace one Operator, but can skip several. This can
be accomplished using the skipRange annotation:
SKIP_RANGE ?= ""

Signed-off-by: Rakshith R rar@redhat.com

Refer: red-hat-storage/odf-operator@e1c41be

cc @nixpanic @Madhu-1 @iamniting

@iamniting
Copy link
Member

@Rakshith-R Did you try to run the make bundle after feeding only one of the values? Does it work well?

@Rakshith-R
Copy link
Member Author

Rakshith-R commented Apr 20, 2022

@Rakshith-R Did you try to run the make bundle after feeding only one of the values? Does it work well?

It fills in empty quotes "" when no value is given.
No errors.

Makefile Outdated
@@ -90,6 +100,8 @@ manifests: controller-gen kustomize ## Generate WebhookConfiguration, ClusterRol
# generate the <package-name>.clusterserviceversion.yaml
config/manifests/bases/$(PACKAGE_NAME).clusterserviceversion.yaml: config/manifests/bases/clusterserviceversion.yaml.in
sed 's/@PACKAGE_NAME@/$(PACKAGE_NAME)/g' < $^ > $@
sed -i 's/@SKIP_RANGE@/$(SKIP_RANGE)/g' $@
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-i is required?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that tells sed to make the replace inplace

@Rakshith-R Rakshith-R requested a review from Madhu-1 April 20, 2022 09:52
@iamniting
Copy link
Member

iamniting commented Apr 20, 2022

@Rakshith-R Did you try to run the make bundle after feeding only one of the values? Does it work well?

It fills in empty quotes "" when no value is given. No errors.

and what about the generated CSV, does it contain the "" quotes, or the key itself is gone?

@Rakshith-R
Copy link
Member Author

Rakshith-R commented Apr 20, 2022

@Rakshith-R Did you try to run the make bundle after feeding only one of the values? Does it work well?

It fills in empty quotes "" when no value is given. No errors.

and what about the generated CSV, does it contain the "" quotes?

Yes, empty quotes.

[rakshith@fedora kubernetes-csi-addons]$ make bundle
....
[rakshith@fedora kubernetes-csi-addons]$ grep skip config/manifests/bases/csi-addons.clusterserviceversion.yaml
olm.skipRange: ""
[rakshith@fedora kubernetes-csi-addons]$ grep replace config/manifests/bases/csi-addons.clusterserviceversion.yaml
replaces: ""

Copy link
Member

@Madhu-1 Madhu-1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes to avoid accidental merge. as still discussion is going on to see as the keys need to be dropped for empty values.

Madhu-1
Madhu-1 previously approved these changes Apr 20, 2022
Each CSV has a replaces parameter that indicates
which Operator it replaces. This builds a graph of
CSVs that can be queried by OLM, and updates can be
shared between channels. Channels can be thought of
as entry points into the graph of updates:
REPLACES ?= ""

Creating the New CatalogSource requires publishing CSVs
that replace one Operator, but can skip several. This can
be accomplished using the skipRange annotation:
SKIP_RANGE ?= ""

Signed-off-by: Rakshith R <rar@redhat.com>
Comment on lines +100 to +103
# generate the <package-name>.clusterserviceversion.yaml base
gen-csv-base:
sed 's/@PACKAGE_NAME@/$(PACKAGE_NAME)/g;s/@SKIP_RANGE@/$(SKIP_RANGE)/g;s/@REPLACES@/$(REPLACES)/g' \
< config/manifests/bases/clusterserviceversion.yaml.in > config/manifests/bases/$(PACKAGE_NAME).clusterserviceversion.yaml
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, changing replace and skip_range variables did not lead to creation of new base csv yaml.

Now, new file is generated each time.

@Rakshith-R Rakshith-R requested a review from Madhu-1 April 20, 2022 10:32
@mergify mergify bot merged commit 786aa36 into csi-addons:main Apr 20, 2022
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.

None yet

4 participants