Add Helm chart and workflow

[YaSuenag]

Pull Request

Issue Number: none, but it relates to #333

Summary

Add and publish Helm chart for Web API

Changes

• Remove samples/helmexample
• Add Helm chart to helm-chart
• Add new workflow publish-helm-chart.yaml to publish the chart to GitHub Container Registry
• Update related documents

Checklist

• Local Tests Passing?
• CICD and Pipeline Tests Passing?
• Added any new Tests?
• Documentation Updates Made?
• Are there any API Changes? If yes, please describe below.
• This is not a breaking change. If it is, please describe it below.

Are there API Changes?

No

Is this a breaking change?

No

[YaSuenag]

I want to discuss about following things:

1. Can we change ADR document without any authorization? or can we consider that it can happen if PR is approved? https://github.com/Green-Software-Foundation/carbon-aware-sdk/pull/381/files#diff-97d76a7bb85785bea9a03183e2a55154faa6c28b5bca4b46b9bba125b6dde626
2. Should new workflow be compliant any naming convention? @danuw https://github.com/Green-Software-Foundation/carbon-aware-sdk/pull/381/files#diff-6de13ed8c4508bc7e0e6447848811f795b32d0cfbdd96d0642c2896b76796307

[YaSuenag]

Update this PR for the installation is more secure:

• Restore service account and security context - the SDK would not require any permissions
• Use application version as a image tag by default
• Use default Kubernetes imagePullPolicy by default
• Disable enableServiceLinks to reduce environment variables

[codecov-commenter]

Codecov Report

Merging #381 (ec06dc7) into dev (b3b3189) will increase coverage by 0.46%.
Report is 21 commits behind head on dev.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##              dev     #381      +/-   ##
==========================================
+ Coverage   74.21%   74.67%   +0.46%     
==========================================
  Files          77       77              
  Lines        2637     2646       +9     
  Branches      266      268       +2     
==========================================
+ Hits         1957     1976      +19     
+ Misses        598      589       -9     
+ Partials       82       81       -1     

see 4 files with indirect coverage changes

[danuw]

Would love to test this so we can get this through. Happy to connect tomorrow? (maybe late Uk time/ morning Japan? (lets slack)

[YaSuenag]

This video is demonstration of the PR. Note that installing the SDK from local directory ( ~/github-forked/carbon-aware-sdk/helm-chart ), not an OCI container.

helm-install.mp4

[danuw]

@YaSuenag as discussed, 2-3 things:

• update documentation to remove changes to the ADR about file structure for helmsamples and add a link to the video above to the relevant documentation on how to use HELM charts
• failing integration tests - we still need to confirm this is an issue on main branch or in here (I assume it is a main branch issue). If it is main branch, we can let that through without fix and fix on main branch directly
• don't forget your DCO

[YaSuenag]

Thanks @danuw for your comment! I reverted the change of ADR, and also added link to the video into overview.md . I hope you can review this PR.

[YaSuenag]

I added empty commit to add DCO in e6e1734 , but DCO action still fails... I hope maintainer updates status manually.

[YaSuenag]

As I wrote in description, this PR contains new workflow "publish-helm-chart.yaml". This workflow expects to kick manually, and publish the chart asynchronously with CASDK release.

In most case, products and its Helm chart is different version. They are distinguished by version and appVersion. https://helm.sh/docs/topics/charts/#charts-and-versioning
In Chart of prom-label-proxy, version is 0.6.0 even though appVersion is v0.7.0. Hence it is ok that Helm chart release would not synchronize CASDK release.

https://github.com/prometheus-community/helm-charts/blob/9f13ae0b7c8bba3ad504b8483606ef5ec2812239/charts/prom-label-proxy/Chart.yaml#L5-L6

Let's think three cases in below:

• Release new CASDK without changing Helm chart: We can use release workflow (kicked automatically by creating new release page)
• Release new Helm chart without CASDK release: Push changes for Helm chart to default branch, then kick publish-helm-chart.yaml manually (workflow_dispatch is triggerd on default branch only)
• Release both CASDK and Helm chart: Release CASDK automatically, and also kick publish-helm-chart.yaml manually

@danuw Let me know if we need to update #252 , and if publish-helm-chart.yaml needs to compliant with some manners in that issue.

[danuw]

Can we include a number in the workflow name so it shows up at the right place in the workflow list - (maybe 4 or 5?)

[YaSuenag]

I think 5 is better because this is not included in release workflow of the SDK.
But I cannot find about 5 in #252 (comment) . Can I use 5?

[YaSuenag]

I gave number 5 to Helm chart in new commit. Let me know if it has problem(s).

[danuw]

Hi @YaSuenag, I tested this and may be related to my lack of knowledge of helm charts at the moment.
I got latest from the PR branch, then ran successively the commands to make sure I expected results like in the video...

what I got was :

 helm-chart % kubectl get pods -n carbon-aware-sdk 
NAME                      READY   STATUS    RESTARTS   AGE
webapi-7655bfd469-q4wmc   1/1     Running   0          5m26s

helm-chart % kubectl get svc -n carbon-aware-sdk
NAME     TYPE       CLUSTER-IP    EXTERNAL-IP   PORT(S)        AGE
webapi   NodePort   10.43.3.203   <none>        80:30438/TCP   7m33s

so I had to change the command to use webapi service instead of the one mentioned. with helm-chart % export NODE_PORT=$(kubectl get --namespace carbon-aware-sdk -o jsonpath="{.spec.ports[0].nodePort}" services webapi)

so finally when i run curl -s "http://192.168.5.15:30438/emissions/bylocation?location=ukwest" it returns nothing ...

@YaSuenag

• Can you help identify what I missed please?
• Also can you add DCO please?

[YaSuenag]

Thanks @danuw for reviewing this!

Can you help identify what I missed please?

Maybe you need to access with hostname which you installed Kubernetes. For example, your Kubernetes runs on "k8shost", you need to test with curl -s "http://k8shost:30438/emissions/bylocation?location=ukwest".

Also can you add DCO please?

I added it in commit e6e1734 , however it does not seem to be detected by DCO workflow. Can you ignore this error?

[YaSuenag]

I updated this PR with 2 commits:

• Fix message for check service port after installation
• Align appVersion with the tag in GHCR (v1.1.0)

@danuw

For example, your Kubernetes runs on "k8shost", you need to test with curl -s "http://k8shost:30438/emissions/bylocation?location=ukwest".

Can you get the result from healthcheck endpoint? curl -v helps us to investigate.

$ curl -sv http://192.168.11.237:32258/health
*   Trying 192.168.11.237:32258...
* Connected to 192.168.11.237 (192.168.11.237) port 32258 (#0)
> GET /health HTTP/1.1
> Host: 192.168.11.237:32258
> User-Agent: curl/8.0.1
> Accept: */*
>
< HTTP/1.1 200 OK
< Content-Type: text/plain
< Date: Sun, 29 Oct 2023 02:24:47 GMT
< Server: Kestrel
< Cache-Control: no-store, no-cache
< Expires: Thu, 01 Jan 1970 00:00:00 GMT
< Pragma: no-cache
< Transfer-Encoding: chunked
<
* Connection #0 to host 192.168.11.237 left intact
Healthy

[vaughanknight]

@danuw to review this PR on another machine - aiming for week of 14 November 2023

[Willmish]

@danuw any updates on this?

[tiwatsuka]

@danuw @YaSuenag
I tested the helm chart on my local cluster.

This is how I tested it:

• checkout the branch
• write a values file to configure the data source
• create the service for CA SDK with the helm chart
• confirm the status of the service and the pod
• port forward to expose the web APIs
• access to the API

I attach the screen captures. They show that the chart works as expected.

[danuw]

Thank you @tiwatsuka for the review with detailed steps.

LGTM - Approved and will override DCO as discussed with @YaSuenag