Feature/helm chart #437

ganievs · 2023-07-27T15:28:07Z

Add the ability to generate the operator's chart from manifests

charts/temporal-operator/Chart.yaml

charts/temporal-operator/templates/deployment.yaml

alexandrevilain · 2023-08-14T20:38:59Z

charts/temporal-operator/templates/selfsigned-issuer.yaml

@@ -0,0 +1,8 @@
+apiVersion: cert-manager.io/v1


I think we could add a condition to create this issuer or not.
Users can choose to not use cert-manager Issuer to create webhook's certs

Add certissue: true in the values file to make it optional.

Why and ? Isn't {{- if .Values.certissue }} enough ?

What do you think about .Values.webhook.certManager.selfSignedIssuer.enabled ?

Good point and what do you think if certissue values will look like this?

certManager: certificate: enabled: true useCustomIssuer: false # Issuer references if you want to use custom issuer # In other case will be used selfSigned issuer issuerRef: {}

charts/temporal-operator/templates/serving-cert.yaml

charts/temporal-operator/values.yaml

yujunz · 2023-08-29T09:16:23Z

@ganievs any update for the pull request? We would like to adopt helm chart for the increasing complexity of multiple temporal operator.

ganievs · 2023-08-29T09:48:26Z

@ganievs any update for the pull request? We would like to adopt helm chart for the increasing complexity of multiple temporal operator.

HI @yujunz! I'm going to make the changes proposed by @alexandrevilain in a few days.

alexandrevilain

Thanks for the update @ganievs !
I still have some comments but it starts to become really cool!

Makefile

alexandrevilain · 2023-09-05T11:21:38Z

Makefile

@@ -169,6 +169,11 @@ artifacts: kustomize
 	$(KUSTOMIZE) build config/crd > ${RELEASE_PATH}/temporal-operator.crds.yaml
 	$(KUSTOMIZE) build config/default > ${RELEASE_PATH}/temporal-operator.yaml

+.PHONY: artifacts
+helm: manifests helmify


As you've overwritten the generated chart, are the new Makefile targets still useful ? IMO we can remove them.
Helmify was just a bootstraping tool, am I wrong ?

Hum this target could still be revelent to push new CRDS to the helm chart when preparing release ?

Seems the helmify is unnecessary in this case. Enough to copy ${RELEASE_PATH}/temporal-operator.crds.yaml to the chart's crds directory.

charts/temporal-operator/Chart.yaml

alexandrevilain · 2023-09-05T11:23:42Z

charts/temporal-operator/templates/selfsigned-issuer.yaml

@@ -0,0 +1,8 @@
+apiVersion: cert-manager.io/v1


Why and ? Isn't {{- if .Values.certissue }} enough ?

alexandrevilain · 2023-09-05T11:24:17Z

charts/temporal-operator/templates/selfsigned-issuer.yaml

@@ -0,0 +1,8 @@
+apiVersion: cert-manager.io/v1


What do you think about .Values.webhook.certManager.selfSignedIssuer.enabled ?

charts/temporal-operator/templates/serving-cert.yaml

alexandrevilain · 2023-09-05T11:25:38Z

charts/temporal-operator/values.yaml

+certissue: true
+imagePullSecrets: []
+kubernetesClusterDomain: cluster.local
+webhookService:


Suggested change

webhookService:

webhook:

service:

What do you think ?

Agree. Rendamed

alexandrevilain · 2023-09-05T11:26:57Z

Once everything is ready, could you please squash your commits ? Thanks :)

alexandrevilain · 2023-09-30T19:49:21Z

Hi @ganievs !
Something went wrong with your squash ? I see only two files to review now

sonarqubecloud · 2023-10-02T06:57:39Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

ganievs · 2023-10-02T06:59:01Z

Hi @ganievs ! Something went wrong with your squash ? I see only two files to review now

Hi, @alexandrevilain sorry for the inconvenience. I forgot to add the chart directory.

bjet007 · 2023-11-09T20:21:39Z

Hi @ganievs, thank's for the contribution. I did the the helm chart with 0.15.4 and before merging the CRD should be updated.

ganievs · 2023-11-13T08:43:26Z

HI @bjet007! Done.

bjet007 · 2023-11-22T20:10:51Z

Hi @ganievs, since 0.16.0 is out, I guess the MR should be updated again to match the CRD?

@alexandrevilain do you have an ETA to merge it back into master and be published at the same time than your release?

bjet007 · 2023-11-22T20:21:53Z

charts/temporal-operator/values.yaml

+    allowPrivilegeEscalation: false
+  image:
+    repository: ghcr.io/alexandrevilain/temporal-operator
+    tag: v0.13.3


The tag here does not match the appVersion bumped to v0.15.4

alexandrevilain · 2023-11-22T20:24:12Z

@alexandrevilain do you have an ETA to merge it back into master and be published at the same time than your release?

Yes sorry, it went under the radar. I have to double check for this PR content again and merge it asap.

bjet007 · 2023-11-22T20:27:54Z

@alexandrevilain do you have an ETA to merge it back into master and be published at the same time than your release?

Yes sorry, it went under the radar. I have to double check for this PR content again and merge it asap.

No problem, found a small issue with the image tag not matching the chart appVersion which lead to running 0.13.0 operator while chart is 0.15.4.

ganievs · 2023-11-27T16:56:59Z

@alexandrevilain do you have an ETA to merge it back into master and be published at the same time than your release?

Yes sorry, it went under the radar. I have to double check for this PR content again and merge it asap.

No problem, found a small issue with the image tag not matching the chart appVersion which lead to running 0.13.0 operator while chart is 0.15.4.

Removed tag definition from values.yaml. Now image tag is the same as appVersion by default

alexandrevilain · 2023-11-28T08:08:04Z

Hi @ganievs !
Could you please rebase and squash your commits ?
Then I'll be able to review your PR :)

Nepoxx · 2023-12-04T16:51:55Z

charts/temporal-operator/templates/deployment.yaml

+      - args: {{- toYaml .Values.manager.args | nindent 8 }}
+        command:
+        - /manager
+        image: {{ .Values.manager.image.repository }}:{{ .Values.manager.image.tag | default .Chart.AppVersion }}


The default .Chart.AppVersion will not work because it lacks the v prefix
i.e. ghcr.io/alexandrevilain/temporal-operator:0.16.0 does not exist

Hi @Nepoxx! Thanks for highlight, fixed

Signed-off-by: Shamil Ganiev <ganiev@pm.me>

ganievs · 2023-12-22T14:18:39Z

Hi @ganievs ! Could you please rebase and squash your commits ? Then I'll be able to review your PR :)

done

sonarqubecloud · 2023-12-22T14:22:12Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

alexandrevilain · 2023-12-24T13:23:01Z

Thanks @ganievs ! This PR is now approved ! Waiting for the CI to pass.
If I remember well this PR provided github actions too ? I can't see it right now, is this a rebase issue ?

ElanHasson · 2023-12-26T13:41:45Z

Congratulations on this great milestone!!

ganievs mentioned this pull request Jul 27, 2023

Helm chart #436

Closed

ganievs force-pushed the feature/helm-chart branch from 5fbfee6 to f07354d Compare August 7, 2023 08:48

alexandrevilain requested changes Aug 14, 2023

View reviewed changes

alexandrevilain requested changes Sep 5, 2023

View reviewed changes

alexandrevilain added the feature label Sep 5, 2023

ganievs force-pushed the feature/helm-chart branch from 898afb0 to 793a994 Compare September 29, 2023 20:29

ganievs force-pushed the feature/helm-chart branch from 793a994 to 8e8029c Compare October 2, 2023 06:57

bjet007 reviewed Nov 22, 2023

View reviewed changes

ganievs force-pushed the feature/helm-chart branch from f3191f0 to cdc6152 Compare November 27, 2023 16:52

Nepoxx reviewed Dec 4, 2023

View reviewed changes

ganievs force-pushed the feature/helm-chart branch 3 times, most recently from 0bcb180 to 06632b8 Compare December 22, 2023 14:14

add helm chart

735824f

Signed-off-by: Shamil Ganiev <ganiev@pm.me>

ganievs force-pushed the feature/helm-chart branch from 06632b8 to be10c2d Compare December 22, 2023 14:18

bump AppVersion in the chart and CRDs

1792dea

ganievs force-pushed the feature/helm-chart branch from be10c2d to 1792dea Compare December 22, 2023 14:21

alexandrevilain approved these changes Dec 24, 2023

View reviewed changes

alexandrevilain merged commit d7c1a39 into alexandrevilain:main Dec 24, 2023
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/helm chart #437

Feature/helm chart #437

ganievs commented Jul 27, 2023

alexandrevilain Aug 14, 2023

ganievs Sep 4, 2023

alexandrevilain Sep 5, 2023

alexandrevilain Sep 5, 2023

ganievs Sep 29, 2023

yujunz commented Aug 29, 2023

ganievs commented Aug 29, 2023 •

edited

Loading

alexandrevilain left a comment

alexandrevilain Sep 5, 2023

alexandrevilain Sep 5, 2023

ganievs Sep 29, 2023

alexandrevilain Sep 5, 2023

alexandrevilain Sep 5, 2023

alexandrevilain Sep 5, 2023

ganievs Sep 29, 2023

alexandrevilain commented Sep 5, 2023 •

edited

Loading

alexandrevilain commented Sep 30, 2023

sonarqubecloud bot commented Oct 2, 2023

ganievs commented Oct 2, 2023

bjet007 commented Nov 9, 2023

ganievs commented Nov 13, 2023

bjet007 commented Nov 22, 2023

bjet007 Nov 22, 2023

alexandrevilain commented Nov 22, 2023

bjet007 commented Nov 22, 2023

ganievs commented Nov 27, 2023

alexandrevilain commented Nov 28, 2023

Nepoxx Dec 4, 2023

ganievs Dec 22, 2023

ganievs commented Dec 22, 2023

sonarqubecloud bot commented Dec 22, 2023

alexandrevilain commented Dec 24, 2023

ElanHasson commented Dec 26, 2023

Feature/helm chart #437

Feature/helm chart #437

Conversation

ganievs commented Jul 27, 2023

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

yujunz commented Aug 29, 2023

ganievs commented Aug 29, 2023 • edited Loading

alexandrevilain 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexandrevilain commented Sep 5, 2023 • edited Loading

alexandrevilain commented Sep 30, 2023

sonarqubecloud bot commented Oct 2, 2023

ganievs commented Oct 2, 2023

bjet007 commented Nov 9, 2023

ganievs commented Nov 13, 2023

bjet007 commented Nov 22, 2023

Choose a reason for hiding this comment

alexandrevilain commented Nov 22, 2023

bjet007 commented Nov 22, 2023

ganievs commented Nov 27, 2023

alexandrevilain commented Nov 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ganievs commented Dec 22, 2023

sonarqubecloud bot commented Dec 22, 2023

Quality Gate passed

alexandrevilain commented Dec 24, 2023

ElanHasson commented Dec 26, 2023

ganievs commented Aug 29, 2023 •

edited

Loading

alexandrevilain commented Sep 5, 2023 •

edited

Loading