knative / operator

Combined operator for Knative.
Apache License 2.0
179 stars 98 forks source link

change to generate kustomize base files from helm chart #1634

Open hhk7734 opened 7 months ago

hhk7734 commented 7 months ago

Fixes #1546

Hi! Changed Helm templates for more flexiblity and added scripts to generate kustomize default files from charts.

When I compared the operator-hub bundles before and after changes, I got the below.

diff -r bundle_old bundle
diff --color -r bundle_old/manifests/knative-operator.v1.6.0.clusterserviceversion.yaml bundle/manifests/knative-operator.v1.6.0.clusterserviceversion.yaml
28c28
<     createdAt: "2023-11-23T13:22:01Z"
---
>     createdAt: "2023-11-23T15:59:22Z"
codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 66.21%. Comparing base (f32bc7c) to head (1f3918d). Report is 16 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1634 +/- ## ========================================== + Coverage 63.54% 66.21% +2.67% ========================================== Files 53 53 Lines 2584 2081 -503 ========================================== - Hits 1642 1378 -264 + Misses 827 588 -239 Partials 115 115 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tynfojlyue commented 6 months ago

I think this is a great change.

houshengbo commented 5 months ago

@hhk7734 Would you mind trying to fix the CI build issues for this PR?

hhk7734 commented 5 months ago

I just rebased to origin/main without changing any code. :)

hhk7734 commented 5 months ago

/test eventing-upgrade-tests

ryan-dyer-sp commented 4 months ago

Can you please include the ability to add custom annotations to all of the objects? Our use case would be to bundle the operator with default knativeserving/eventing objects along with the kafka eventing controller within an argocd application and we would need to control install order via argo's sync-wave annotations. The kafka eventing controller does not offer a helm chart so we only have a manifest file which would run at a default order of 0. So the ability to say the operator's sync-wave itself is <0 would be great.

hhk7734 commented 4 months ago

Hi! @ryan-dyer-sp

Can you please include the ability to add custom annotations to all of the objects? Our use case would be to bundle the operator with default knativeserving/eventing objects along with the kafka eventing controller within an argocd application and we would need to control install order via argo's sync-wave annotations. The kafka eventing controller does not offer a helm chart so we only have a manifest file which would run at a default order of 0. So the ability to say the operator's sync-wave itself is <0 would be great.

I plan to add patterns that are frequently seen in helm values, as shown below. However, if there are too many changes in this PR, the review will be difficult, so I think it would be better to proceed with the follow-up work once this PR is merged.

commonLabels: {}

operator:
  labels: {}
  annotations: {}
  podLabels: {}

There are still a few namespace: default references. Shouldnt these all be namespace: {{ include "knativeOperator.namespace" .}}

Files with namespace: default are not in config/charts/. These are the base files for kustomize.

knative-prow[bot] commented 2 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hhk7734 Once this PR has been reviewed and has the lgtm label, please assign psschwei for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/knative/operator/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment