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

Support location source setting in Helm chart #449

Merged

Conversation

YaSuenag
Copy link
Contributor

Pull Request

Summary

Add location source configuration to Helm chart

Changes

  • Add locationSources key to configure location source JSON.
  • Ignore files which starts with .. in location-source directory (Kubernetes sets real configuration file with ..)
  • Update documents for Helm

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

Anything else?

I want to update Helm chart version and to publish new chart container if this PR is accepted.

Signed-off-by: Yasumasa Suenaga <suenaga@oss.nttdata.com>
@vaughanknight vaughanknight added the 1.3 Tracking the work towards release 1.3 label Jan 30, 2024
@tiwatsuka
Copy link
Contributor

@vaughanknight @danuw @YaSuenag
I have validated this PR. The changes look good for me and it runs expectedly.

Configuration:

locationSources:
  enabled: enable
  files:
  - fileName: custom-locations-1.json
    locations: |-
      {
        "east": {
          "Latitude": "35.68",
          "Longitude": "139.77",
          "Name": "eastdc"
        },
        "west": {
          "Latitude": "34.6939",
          "Longitude": "135.5022",
          "Name": "westdc"
        }
      }
  - fileName: custom-locations-2.json
    locations: |-
      {
        "north": {
          "Latitude": "35.68",
          "Longitude": "139.77",
          "Name": "northdc"
        },
        "south": {
          "Latitude": "34.6939",
          "Longitude": "135.5022",
          "Name": "southdc"
        }
      }

After executing helm install:

$ kubectl describe configmap location-sources -n carbon-aware-sdk
Name:         location-sources
Namespace:    carbon-aware-sdk
Labels:       app.kubernetes.io/instance=casdk
              app.kubernetes.io/managed-by=Helm
              app.kubernetes.io/name=carbon-aware-sdk
              app.kubernetes.io/version=v1.1.0
              helm.sh/chart=carbon-aware-sdk-1.0.0
Annotations:  meta.helm.sh/release-name: casdk
              meta.helm.sh/release-namespace: carbon-aware-sdk

Data
====
custom-locations-2.json:
----
{
  "north": {
    "Latitude": "35.68",
    "Longitude": "139.77",
    "Name": "northdc"
  },
  "south": {
    "Latitude": "34.6939",
    "Longitude": "135.5022",
    "Name": "southdc"
  }
}
custom-locations-1.json:
----
{
  "east": {
    "Latitude": "35.68",
    "Longitude": "139.77",
    "Name": "eastdc"
  },
  "west": {
    "Latitude": "34.6939",
    "Longitude": "135.5022",
    "Name": "westdc"
  }
}

BinaryData
====

Events:  <none>
$ kubectl get po -n carbon-aware-sdk -o wide
NAME                      READY   STATUS    RESTARTS   AGE     IP             NODE           NOMINATED NODE   READINESS GATES
webapi-6c8d49cf58-7h5bx   1/1     Running   0          7m55s   10.1.129.156   host-worker-1  <none>           <none>
$ curl http://10.1.129.156/locations 
{"east":{"latitude":35.68,"longitude":139.77,"name":"eastdc"},"west":{"latitude":34.6939,"longitude":135.5022,"name":"westdc"},"north":{"latitude":35.68,"longitude":139.77,"name":"northdc"},"south":{"latitude":34.6939,"longitude":135.5022,"name":"southdc"}}

Signed-off-by: Yasumasa Suenaga <suenaga@oss.nttdata.com>
@YaSuenag
Copy link
Contributor Author

I set new version (1.1.0) for Helm chart in this PR.

As I wrote in #381 , Helm chart release can happen asynchronously with CASDK release. So I hope new Helm chart will be published after merging this PR - it means CASDK mainteiner kicks https://github.com/Green-Software-Foundation/carbon-aware-sdk/blob/dev/.github/workflows/5-publish-helm-chart.yaml manually.

@danuw
Copy link
Collaborator

danuw commented Feb 13, 2024

Thanks both and @tiwatsuka for the review and test confirmation

@danuw danuw merged commit b45c456 into Green-Software-Foundation:dev Feb 13, 2024
9 checks passed
@YaSuenag YaSuenag deleted the pr/helm-location-sources branch February 13, 2024 00:50
@YaSuenag
Copy link
Contributor Author

Thank you for merging!

Could you kick 5-publish-helm-chart.yaml to publish this chart?

@YaSuenag
Copy link
Contributor Author

@danuw danuw mentioned this pull request Feb 26, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.3 Tracking the work towards release 1.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants