Fix the wrong version of container tag for Radius installation #5628
Conversation
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Test Results2 750 tests +2 2 743 ✔️ +2 1m 55s ⏱️ -12s Results for commit f2f8d71. ± Comparison against base commit c3b94cc. This pull request removes 2 and adds 4 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
For the detailed report, please go to
|
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
nithyatsu
force-pushed
the
imageversionfix
branch
from
f54a7d8 to
0b8baf1
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
For the detailed report, please go to
|
pkg/cli/helm/radiusclient.go
Outdated
| services := []string{"rp", "ucp", "de"} | ||
| for _, service := range services { | ||
| _, ok := values[service] | ||
| if !ok { | ||
| values[service] = make(map[string]any) | ||
| } | ||
| serviceconfig := values[service].(map[string]any) | ||
| serviceconfig["tag"] = options.ImageVersion | ||
| } |
There was a problem hiding this comment.
no one set values to rp, ucp, and de. So we can simplify the code like below.
| services := []string{"rp", "ucp", "de"} | |
| for _, service := range services { | |
| _, ok := values[service] | |
| if !ok { | |
| values[service] = make(map[string]any) | |
| } | |
| serviceconfig := values[service].(map[string]any) | |
| serviceconfig["tag"] = options.ImageVersion | |
| } | |
| for _, service := range []string{"rp", "ucp", "de"} { | |
| values[service] = map[string]any{ | |
| "tag": options.ImageVersion | |
| } | |
| } |
There was a problem hiding this comment.
actually, at this point we have the default values populated from values.yaml, on top of it we add the tag and then later overwrite if -- set is specified.. therefore I couldnt take this approach and make a new map.. I will remove the code from 122-125.
There was a problem hiding this comment.
Ah got it. I would keep L122-L125 as is. Let's make it simpler with shorter variable names like below.
| services := []string{"rp", "ucp", "de"} | |
| for _, service := range services { | |
| _, ok := values[service] | |
| if !ok { | |
| values[service] = make(map[string]any) | |
| } | |
| serviceconfig := values[service].(map[string]any) | |
| serviceconfig["tag"] = options.ImageVersion | |
| } | |
| services := []string{"rp", "ucp", "de"} | |
| for _, service := range services { | |
| if _, ok := values[service]; !ok { | |
| values[service] = map[string]any{} | |
| } | |
| o := values[service].(map[string]any) | |
| o["tag"] = options.ImageVersion | |
| } |
pkg/cli/helm/radiusclient.go
Outdated
| @@ -114,6 +113,28 @@ func ApplyRadiusHelmChart(options RadiusOptions, kubeContext string) (bool, erro | |||
| return alreadyInstalled, err | |||
| } | |||
|
|
|||
| func AddRadiusValues(helmChart *chart.Chart, options *RadiusOptions) error { | |||
|
|
|||
pkg/cli/helm/radiusclient_test.go
Outdated
| _, ok := values["rp"] | ||
|
|
||
| assert.True(t, ok) | ||
| rp := values["rp"].(map[string]any) |
There was a problem hiding this comment.
ucp and de need to be validated too
nithyatsu
changed the title
.vscode/launch.json
Outdated
| @@ -44,7 +44,10 @@ | |||
| "preLaunchTask": "Build Radius (all)", | |||
| "program": "${workspaceFolder}/cmd/rad/main.go", | |||
| "cwd": "${workspaceFolder}", | |||
| "args": [], | |||
| "args": [ | |||
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
Can you please rename the title to |
|
For the detailed report, please go to
|
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
@nithyatsu to help with the release notes generation could you please rename this PR to be self-describing without a reference to the issue (that can go in the description). Maybe something like: "Fix CLI Helm versioning". Thanks!! |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
For the detailed report, please go to
|
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
For the detailed report, please go to
|
nithyatsu
force-pushed
the
imageversionfix
branch
from
162f1ab to
f2f8d71
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
@youngbupark @nithyatsu could we drop the [Hotfix] prefix? Messes with our PR docs generation when we release later. |
|
For the detailed report, please go to
|
Ok I didn't know it. |
nithyatsu
changed the title
# Description Hotfix to Add logic to use the versioned image. After the fix: ``` kubectl cluster-info --context kind-kind Have a nice day! 👋 nithya@Nithyas-MacBook-Pro radius % rad version RELEASE VERSION BICEP COMMIT 0.21.0 show-75-gb257abe 0.11.51 b257abe nithya@Nithyas-MacBook-Pro radius % rad init Initializing Radius... ✅ Install Radius show-75-gb257abe - Kubernetes cluster: kind-kind - Kubernetes namespace: radius-system ✅ Create new environment default - Kubernetes namespace: default ✅ Scaffold application radius ✅ Update local configuration Initialization complete! Have a RAD time 😎 nithya@Nithyas-MacBook-Pro radius % k describe pod -n radius-system ucp-7788d965c4-f2jdp Name: ucp-7788d965c4-f2jdp Namespace: radius-system Priority: 0 Service Account: ucp Node: kind-control-plane/172.18.0.2 Start Time: Fri, 02 Jun 2023 10:12:23 -0700 Labels: app.kubernetes.io/part-of=radius-control-plane control-plane=ucp pod-template-hash=7788d965c4 Annotations: prometheus.io/path: /metrics prometheus.io/port: 9090 prometheus.io/scrape: true Status: Running IP: 10.244.0.5 IPs: IP: 10.244.0.5 Controlled By: ReplicaSet/ucp-7788d965c4 Containers: ucp: Container ID: containerd://383d5bd2dd443ee68752cb3b78b88ae520ce4124530d3a34f6947b67121d2136 Image: **radius.azurecr.io/ucpd:0.21** Image ID: radius.azurecr.io/ucpd@sha ``` ``` nithya@Nithyas-MacBook-Pro radius % k describe pod -n radius-system bicep-de-fd7d9749c-42wf7 Name: bicep-de-fd7d9749c-42wf7 Namespace: radius-system Priority: 0 Service Account: de-manager Node: kind-control-plane/172.18.0.2 Start Time: Fri, 02 Jun 2023 10:12:23 -0700 Labels: app.kubernetes.io/part-of=radius-control-plane control-plane=de pod-template-hash=fd7d9749c Annotations: prometheus.io/path: /metrics prometheus.io/port: 6443 prometheus.io/scrape: true Status: Running IP: 10.244.0.6 IPs: IP: 10.244.0.6 Controlled By: ReplicaSet/bicep-de-fd7d9749c Containers: de: Container ID: containerd://6ffc6acac691e411ec4b3a48c6e18a496a50f3781eb69ad72e9a3d35e49cf4fc Image: radius.azurecr.io/deployment-engine:0.21 Image ID: radius.azurecr.io/deployment-engine@sha ``` ``` nithya@Nithyas-MacBook-Pro radius % k describe pod -n radius-system bicep-de-fd7d9749c-42wf7 Name: bicep-de-fd7d9749c-42wf7 Namespace: radius-system Priority: 0 Service Account: de-manager Node: kind-control-plane/172.18.0.2 Start Time: Fri, 02 Jun 2023 10:12:23 -0700 Labels: app.kubernetes.io/part-of=radius-control-plane control-plane=de pod-template-hash=fd7d9749c Annotations: prometheus.io/path: /metrics prometheus.io/port: 6443 prometheus.io/scrape: true Status: Running IP: 10.244.0.6 IPs: IP: 10.244.0.6 Controlled By: ReplicaSet/bicep-de-fd7d9749c Containers: de: Container ID: containerd://6ffc6acac691e411ec4b3a48c6e18a496a50f3781eb69ad72e9a3d35e49cf4fc Image: radius.azurecr.io/deployment-engine:0.21 Image ID: radius.azurecr.io/deployment-engine@sha256:e312d9836a0b278eb3e43fa074258d9e6981d5e8e3c7adb0d3267f19f99b961f Port: 6443/TCP Host Port: 0/TCP Args: --kubernetes=true State: Running Started: Fri, 02 Jun 2023 10:12:46 -0700 ``` ``` nithya@Nithyas-MacBook-Pro radius % k get pods -n radius-system NAME READY STATUS RESTARTS AGE appcore-rp-7c566fd5f5-lmhj6 1/1 Running 0 6m11s bicep-de-fd7d9749c-42wf7 1/1 Running 0 6m11s contour-contour-7c88c58fc8-6wklr 1/1 Running 0 5m10s contour-envoy-4wvtq 2/2 Running 0 5m10s ucp-7788d965c4-f2jdp 1/1 Running 0 6m11s ``` ## Issue reference Fixes: #5624 ## Checklist Please make sure you've completed the relevant tasks for this PR, out of the following list: * [ ] Code compiles correctly * [ ] Adds necessary unit tests for change * [ ] Adds necessary E2E tests for change * [ ] Unit tests passing * [ ] Extended the documentation / Created issue for it ## Auto-generated summary <!-- GitHub Copilot for docs will auto-generate a summary of the PR --> <!-- copilot:all --> ### <samp>🤖 Generated by Copilot at 0e9ded4</samp> ### Summary 🎛️🖼️🛠️ <!-- 1. 🎛️ - This emoji represents the radius CLI and the ability to control the image version for the radius services. It suggests a sense of configuration and customization. 2. 🖼️ - This emoji represents the image version and the helm chart. It suggests a sense of visualizing and updating the software components. 3. 🛠️ - This emoji represents the refactoring and error handling improvements. It suggests a sense of fixing and enhancing the code quality. --> This pull request enables the radius CLI to control the image version of the radius services deployed by the helm chart. It modifies `pkg/cli/helm/cluster.go` and `pkg/cli/helm/radiusclient.go` to pass and use the `tag` variable. > _To set the image version for radius_ > _You can use the CLI with a `tag` fuss_ > _Or the helm chart with ease_ > _And a helper, if you please_ > _But don't forget to handle errors and format thus_ ### Walkthrough * Add `ImageVersion` field to `ClusterOptions` and `RadiusOptions` structs to store the image version for the radius services ([link](https://github.com/project-radius/radius/pull/5628/files?diff=unified&w=0#diff-a94c29c96fb0778e35d7983669dedfa10de7111040707a3839dace9836b20d0bR64), [link](https://github.com/project-radius/radius/pull/5628/files?diff=unified&w=0#diff-7f93f7a0f0c6c2329155f30e6a4cc6928277acabefab4c28826cb143a23ce90eR46)) * Modify `NewDefaultClusterOptions` function to set the `ImageVersion` based on the version channel of the radius CLI ([link](https://github.com/project-radius/radius/pull/5628/files?diff=unified&w=0#diff-a94c29c96fb0778e35d7983669dedfa10de7111040707a3839dace9836b20d0bR53-R57)) * Create `AddRadiusValues` function in `helm` package to add the image version to the helm chart values and parse the additional values from the `options.Values` field ([link](https://github.com/project-radius/radius/pull/5628/files?diff=unified&w=0#diff-7f93f7a0f0c6c2329155f30e6a4cc6928277acabefab4c28826cb143a23ce90eR116-R134)) * Use `AddRadiusValues` function in `ApplyRadiusHelmChart` function instead of `strvals.ParseInto` and add error handling and formatting ([link](https://github.com/project-radius/radius/pull/5628/files?diff=unified&w=0#diff-7f93f7a0f0c6c2329155f30e6a4cc6928277acabefab4c28826cb143a23ce90eL75-R78)) (cherry picked from commit 381c73f)
# Description Including changes in [PR](#5628) for Release v0.21 per issue described in #5624 Fixes: 5624 ## Checklist Please make sure you've completed the relevant tasks for this PR, out of the following list: * [ ] Code compiles correctly * [ ] Adds necessary unit tests for change * [ ] Adds necessary E2E tests for change * [ ] Unit tests passing * [ ] Extended the documentation / Created issue for it ## Auto-generated summary <!-- copilot:all --> ### <samp>🤖 Generated by Copilot at bd4f105</samp> ### Summary 🧪📝🚀 <!-- 1. 🧪 for adding unit tests to the helm package 2. 📝 for updating the documentation and validation workflow 3. 🚀 for adding support for image version channels and refactoring the helm chart values parsing --> This pull request updates the bicep validation workflow and adds support for specifying image version based on version channel for radius cluster and service creation. It also refactors the helm chart values parsing and adds unit tests for the new function. The changes affect the `.github/workflows/validate-bicep.yaml`, `pkg/cli/helm/cluster.go`, `pkg/cli/helm/radiusclient.go`, and `pkg/cli/helm/radiusclient_test.go` files. > _We're setting sail with `radius` on the helm_ > _We've added tests and options for the image tag_ > _We've updated the workflow with the latest tools_ > _So heave away, me hearties, heave away_ ### Walkthrough * Update the refs and URL for the docs, samples, and rad-bicep-corerp tool to use the stable release branch and version ([link](https://github.com/project-radius/radius/pull/5633/files?diff=unified&w=0#diff-4f41a75886a1a479805bae1b7fb335479735aa7740e02b9e5c19992ead18f73eL40-R40), [link](https://github.com/project-radius/radius/pull/5633/files?diff=unified&w=0#diff-4f41a75886a1a479805bae1b7fb335479735aa7740e02b9e5c19992ead18f73eL47-R47), [link](https://github.com/project-radius/radius/pull/5633/files?diff=unified&w=0#diff-4f41a75886a1a479805bae1b7fb335479735aa7740e02b9e5c19992ead18f73eL60-R60)) * Add the tag variable and the ImageVersion field to the ClusterOptions struct and the NewDefaultClusterOptions function in `cluster.go` to determine the image tag for the radius cluster based on the version channel ([link](https://github.com/project-radius/radius/pull/5633/files?diff=unified&w=0#diff-a94c29c96fb0778e35d7983669dedfa10de7111040707a3839dace9836b20d0bR51-R55), [link](https://github.com/project-radius/radius/pull/5633/files?diff=unified&w=0#diff-a94c29c96fb0778e35d7983669dedfa10de7111040707a3839dace9836b20d0bR62)) * Add the ImageVersion field to the RadiusOptions struct in `radiusclient.go` to allow overriding the default image tag for the radius services ([link](https://github.com/project-radius/radius/pull/5633/files?diff=unified&w=0#diff-7f93f7a0f0c6c2329155f30e6a4cc6928277acabefab4c28826cb143a23ce90eL43-R47)) * Add the AddRadiusValues function to the helm package and call it in `radiusclient.go` to add the image version and the options.Values to the helm chart values in a specific order of priority ([link](https://github.com/project-radius/radius/pull/5633/files?diff=unified&w=0#diff-7f93f7a0f0c6c2329155f30e6a4cc6928277acabefab4c28826cb143a23ce90eL69-R76), [link](https://github.com/project-radius/radius/pull/5633/files?diff=unified&w=0#diff-7f93f7a0f0c6c2329155f30e6a4cc6928277acabefab4c28826cb143a23ce90eR114-R137)) * Capitalize the error messages for consistency in `radiusclient.go` ([link](https://github.com/project-radius/radius/pull/5633/files?diff=unified&w=0#diff-7f93f7a0f0c6c2329155f30e6a4cc6928277acabefab4c28826cb143a23ce90eL58-R60), [link](https://github.com/project-radius/radius/pull/5633/files?diff=unified&w=0#diff-7f93f7a0f0c6c2329155f30e6a4cc6928277acabefab4c28826cb143a23ce90eL98-R98), [link](https://github.com/project-radius/radius/pull/5633/files?diff=unified&w=0#diff-7f93f7a0f0c6c2329155f30e6a4cc6928277acabefab4c28826cb143a23ce90eL105-R105)) * Add the `radiusclient_test.go` file and two unit tests for the AddRadiusValues function using the testify library ([link](https://github.com/project-radius/radius/pull/5633/files?diff=unified&w=0#diff-d86037fd6c18ac1d33c42f14b22694ed549f413bc6f01e486edcc069e3e50e08R1-R110)) --------- Co-authored-by: nithyatsu <[email protected]>
Description
Hotfix to Add logic to use the versioned image.
After the fix:
Issue reference
Fixes: radius-project/core-team#1200
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list:
Auto-generated summary
🤖 Generated by Copilot at 0e9ded4
Summary
🎛️🖼️🛠️
This pull request enables the radius CLI to control the image version of the radius services deployed by the helm chart. It modifies
pkg/cli/helm/cluster.goandpkg/cli/helm/radiusclient.goto pass and use thetagvariable.Walkthrough
ImageVersionfield toClusterOptionsandRadiusOptionsstructs to store the image version for the radius services (link, link)NewDefaultClusterOptionsfunction to set theImageVersionbased on the version channel of the radius CLI (link)AddRadiusValuesfunction inhelmpackage to add the image version to the helm chart values and parse the additional values from theoptions.Valuesfield (link)AddRadiusValuesfunction inApplyRadiusHelmChartfunction instead ofstrvals.ParseIntoand add error handling and formatting (link)