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

Ossm merge #112

Merged
merged 11 commits into from
Dec 13, 2022
Merged

Ossm merge #112

merged 11 commits into from
Dec 13, 2022

Conversation

alexsnaps
Copy link
Member

No description provided.

didierofrivia and others added 2 commits December 8, 2022 10:37
* Bumps opm version
* Creates tasks to load olm needed images to Kind for local testing
* Uses custom catalog Dockerfile

[catalog] Custom Dockerfile for building catalog

* In order to use instead of autogenerated index.Dockerfile
* To mitigate operator-framework/operator-registry#619

[gh] Using custom catalog.Dockerfile to build catalog images

[gh] Adding platforms for building images

[makefile] Adding platform param in custom docker build command
@alexsnaps alexsnaps requested a review from a team as a code owner December 8, 2022 15:39
@@ -186,7 +186,7 @@ act: $(ACT) ## Download act locally if necessary.

.PHONY: manifests
manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./api/v1beta1" output:crd:artifacts:config=config/crd/bases
Copy link
Member Author

Choose a reason for hiding this comment

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

?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's because of the external API defs now hosted in our code base. We don't want them in the output generated manifests.

@@ -69,6 +69,9 @@ jobs:
- name: Run make bundle
if: ${{ github.ref_name != env.MAIN_BRANCH_NAME }}
run: make bundle REGISTRY=${{ env.IMG_REGISTRY_HOST }} ORG=${{ env.IMG_REGISTRY_ORG }} IMAGE_TAG=${{ github.ref_name }}
- name: Run make bundle (main)
Copy link
Contributor

Choose a reason for hiding this comment

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

why make bundle twice?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's the only thing left unaddressed… Am not sure where this whole IMAGE_TAG=latest VERSION=0.0.0 thing is coming from or what it addresses, @didierofrivia Can you shed some light?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I got this.

The key is the conditional if.

  • if: ${{ github.ref_name != env.MAIN_BRANCH_NAME }} -> The bundle will reference kuadrant operator github.ref_name
  • if: ${{ github.ref_name == env.MAIN_BRANCH_NAME }} -> The bundle will reference kuadrant operator latest (instead of main). BTW both latest and main are created and pushed to quay.

For me, specifying the VERSION here is useless, as the default value will be 0.0.0. I think this is just to make sure latest bundle has always ''0.0.0` version, no matter what is specified in the Makefile.

For the case of the bundle, the proposed changes are ok to me, although not very relevant. Thus, the bundle will reference latest with these changes and otherwise, the referenced image will be main.

.github/workflows/build-images.yaml Outdated Show resolved Hide resolved
.github/workflows/build-images.yaml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
catalog.Dockerfile Outdated Show resolved Hide resolved
Copy link
Collaborator

@didierofrivia didierofrivia left a comment

Choose a reason for hiding this comment

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

It looks good to me, except for the changes leaked from #105. I could fix the merge and changes requested by Eguz too if it proves tedious.

.github/workflows/build-images.yaml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@alexsnaps
Copy link
Member Author

There are a few things unaddressed still, @didierofrivia could you please have a look? You can either push one (or more) commits to this branch or just advise what you think might be best on moving forward… Thanks 🙏

* The catalog pod has its image policy set to ALWAYS
@alexsnaps alexsnaps merged commit c940ead into main Dec 13, 2022
@alexsnaps alexsnaps deleted the OSSM-merge branch December 13, 2022 15:17
mikenairn pushed a commit to mikenairn/kuadrant-operator that referenced this pull request Mar 23, 2023
* utilise wasm as the integration point (Kuadrant#111)

* utilise wasm as the integration point

* increase no. of retries

* update broken manifests

* fetch operations from virtualservice (Kuadrant#112)

* fetch operation from virtualservice

* use operations instead of 'operation'

* update sha256 checksum

* update wasm module sha256 checksum (Kuadrant#113)

* Handle httproute signal from ratelimitpolicy controller (Kuadrant#116)

* handle httproute signal from rlp controller

* send signal to RLP from route reconciler

* remove hosts field from SignalingNetwork

* add comment over SendSignal

* fix signal trigger in routing resources

* remove hosts from RLP operations

* bring back EnvoyFilter for route controller
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