Skip to content

Conversation

@sgissi
Copy link

@sgissi sgissi commented Nov 23, 2025

Description

This PR supersedes #408, to add a new blog post about the new official Valkey Helm Chart, including reasons for the chart creation and migration steps from the existing chart.

Issues Resolved

Closes #408

Check List

  • [ X ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the BSD-3-Clause License.

cherukum-Amazon and others added 2 commits November 18, 2025 22:45
Signed-off-by: cherukum-amazon <cherukum@amazon.com>
Signed-off-by: Silvio Gissi <silvio@gissilabs.com>
@sgissi
Copy link
Author

sgissi commented Nov 23, 2025

@cherukum-Amazon @stockholmux Can you take a look at this update please? It has a dependency on a new release of valkey-helm supporting replicas (valkey-io/valkey-helm#84) which I'm merging shortly.

OK
$ $NEW_VALKEY_CLI info | grep '^\(role\|master_host\|master_link_status\)'
role:slave
master_host:valkey-bitnami-primary
Copy link

@cherukum-Amazon cherukum-Amazon Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the instance name, can we map to an existing environment variable?.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also keep environment variables in one place with comments, so its easy to back track.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shell variable here is misleading. I'll update to create a shell alias instead.

Update all clients to use the new Valkey read-write and read-only endpoints which are exposed as services (`SERVICE.NAMESPACE.svc.cluster.local`). In the example above:

* Read-Write (primary): `valkey.apps-test.svc.cluster.local`
* Read-Only (all instances): `valkey-read.apps-test.svc.cluster.local`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we tag the endpoints to environment variables?.


```shell
$ kubectl get pods --all-namespaces -l app.kubernetes.io/name=valkey -o custom-columns=Pod:.metadata.name,Namespace:.metadata.namespace,Instance:.metadata.labels.app\\.kubernetes\\.io\\/instance
Pod Namespace Instance
Copy link

@cherukum-Amazon cherukum-Amazon Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is 53 to 57 sample outcome from the command?. can we call it out explicitly with a comment

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated all shells where output is shown with # * Sample Output *, not sure it is clearer or not. By default if a shell command is listed with a $ in front, it means to show the shell command and output.

$ $NEW_VALKEY_CLI replicaof $SVCPRIMARY 6379
OK
$ $NEW_VALKEY_CLI info | grep '^\(role\|master_host\|master_link_status\)'
role:slave

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this statement do?.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding comments might help.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

@stockholmux stockholmux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the author bio then I can review :)

Signed-off-by: Silvio Gissi <silvio@gissilabs.com>
Copy link
Member

@stockholmux stockholmux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall good.

A few little issues with shell vs bash, using $, and not commenting example output. In general, this is so if someone copy/pastes via double click everything will actually run in a shell instead of trying to execute the sample output.


### Step 1: Find existing pods, services and namespace

```shell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shell as a tag block causes Zola to spam the following warning: Warning: Highlight language shell not found in blog/2025-11-05-valkey-helm-chart/index.md - it really should be bash (and you'll get syntax highlighting too)


```shell
# List all pods in all namespaces with app name 'valkey'
$ kubectl get pods --all-namespaces -l app.kubernetes.io/name=valkey -o custom-columns=Pod:.metadata.name,Namespace:.metadata.namespace,Instance:.metadata.labels.app\\.kubernetes\\.io\\/instance
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop the $

# List all pods in all namespaces with app name 'valkey'
$ kubectl get pods --all-namespaces -l app.kubernetes.io/name=valkey -o custom-columns=Pod:.metadata.name,Namespace:.metadata.namespace,Instance:.metadata.labels.app\\.kubernetes\\.io\\/instance
# * Sample Output *
Pod Namespace Instance
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment out all example output (or put it in a different code block).


Install the new Valkey instance:

```shell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shell->bash


Check it is running as expected:

```shell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bash


```shell
# List new pods and ensure they are in 'Running' state
$ kubectl get pods -n $NAMESPACE -l app.kubernetes.io/instance=$NEWINSTANCE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no $

# List new pods and ensure they are in 'Running' state
$ kubectl get pods -n $NAMESPACE -l app.kubernetes.io/instance=$NEWINSTANCE
# * Sample Output *
NAME READY STATUS RESTARTS AGE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

example output should be commented

Signed-off-by: Silvio Gissi <silvio@gissilabs.com>
@sgissi
Copy link
Author

sgissi commented Dec 2, 2025

@mk-raven This is a blog post about migrating from Bitnami's Valkey chart into the official one, feel free to chime in here if you have any comments.

@sgissi
Copy link
Author

sgissi commented Dec 2, 2025

@stockholmux @cherukum-Amazon Post updated. PR valkey-io/valkey-helm#84 needs to be merged and a new chart released before publishing.

Copy link

@cherukum-Amazon cherukum-Amazon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks Silvio for taking the time.

helm repo update
```

Create a `values.yaml` file that matches your current deployment. The example below is similar to the default Bitnami Valkey configuration:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the end of the sentence, can we add a link to README/documentation, which authors can reference if they want to configure values.yaml with additional configuration - a) PVC storage for AOF, b) TLS c) Auth d) metrics

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a commend to refer to the README and chart values.yaml for details. PVC storage is enabled by default but AOF/RDB isn't, I added the control over persistence item in the "What's Next" with a reference to the open issue. Thanks for the review!

Copy link
Member

@stockholmux stockholmux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with a nit. Fix if you're making another commit, otherwise LGTM.

# List all pods in all namespaces with app name 'valkey'
kubectl get pods --all-namespaces -l app.kubernetes.io/name=valkey -o custom-columns=Pod:.metadata.name,Namespace:.metadata.namespace,Instance:.metadata.labels.app\\.kubernetes\\.io\\/instance
# * Sample Output *
#Pod Namespace Instance
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the spacing between the # the sample text is inconsistent. If you make more changes to the post, fix that. I won't hold merge on that though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it looks better indeed with a space in front. Updated now.

@stockholmux
Copy link
Member

@sgissi Since this is dependent on valkey-io/valkey-helm#84, please swap over to a draft PR. That will prevent accidental merging given that I've already approved the post.

Signed-off-by: Silvio Gissi <silvio@gissilabs.com>
@sgissi sgissi marked this pull request as draft December 2, 2025 18:19
@sgissi
Copy link
Author

sgissi commented Dec 2, 2025

@sgissi Since this is dependent on valkey-io/valkey-helm#84, please swap over to a draft PR. That will prevent accidental merging given that I've already approved the post.

Good point, marked as draft until the PR is merged and the new chart is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants