Skip to content

fix: apps not using desired storageclass in linode#3058

Merged
ElderMatt merged 39 commits intomainfrom
APL-1569
Apr 1, 2026
Merged

fix: apps not using desired storageclass in linode#3058
ElderMatt merged 39 commits intomainfrom
APL-1569

Conversation

@ElderMatt
Copy link
Copy Markdown
Contributor

This PR fixes the storage class being used for gitea-valkey and oauth2-proxy, it is now set to linode-block-storage instead of linode-block-storage-retain. Migration has also been added to delete and recreate the pvcs.

@svcAPLBot
Copy link
Copy Markdown
Contributor

svcAPLBot commented Mar 18, 2026

Comparison of Helm chart templating output:

@@ spec.volumeClaimTemplates.v1/PersistentVolumeClaim/valkey-data.spec @@
! + one map entry added:
+ storageClassName: linode-block-storage


@@ spec.volumeClaimTemplates.v1/PersistentVolumeClaim/data.spec @@
! + one map entry added:
+ storageClassName: linode-block-storage


@@ data.VERSIONS @@
! ± value change in multiline text (one insert, one deletion)
  
- {"api":"main","aplCharts":"main","console":"main","consoleLogin":"main","core":"main","specVersion":58,"tasks":"main","tools":"main"}
+ {"api":"main","aplCharts":"main","console":"main","consoleLogin":"main","core":"main","specVersion":59,"tasks":"main","tools":"main"}




@@ versions.specVersion @@
! ± value change
- 58
+ 59

@merll
Copy link
Copy Markdown
Contributor

merll commented Mar 19, 2026

I am fine with the main functionality of this branch, but it contains lots of unrelated changes which make it harder to read and later follow up in case someone is looking into old code. I also doubt such changes can be properly QA'ed when they are not used in any recent environment.

I would suggest either

  • reverting the unrelated changes
  • or removing old migrations entirely. The Istio migration is from 9 months ago and if someone still needs to upgrade a cluster this old, it should be done in smaller increments.

@ElderMatt ElderMatt requested a review from merll March 23, 2026 13:34
Copy link
Copy Markdown
Contributor

@merll merll left a comment

Choose a reason for hiding this comment

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

I am fine with the changes if they solve the issue. What I was wondering about when reading this: We already set defaultStorageClass according to the provider linode. Did we find out why this was not used?
I am asking because I would like to avoid hiding a possibly more essential issue hidden by this patch.

@ElderMatt
Copy link
Copy Markdown
Contributor Author

We did not set the storageClass for gitea-valkey which is why it was defaulting to linode-block-storage-retain. For Oauth2 I'm not quite sure.

@ElderMatt ElderMatt merged commit 1e0a51d into main Apr 1, 2026
15 checks passed
@ElderMatt ElderMatt deleted the APL-1569 branch April 1, 2026 08:33
@ElderMatt
Copy link
Copy Markdown
Contributor Author

ElderMatt commented Apr 1, 2026

It seems for older cluster before spec version 44 we never set the defaultStorageClass so it is ''. In specVersion 44 we added the defaultStorageClass but there was no migration to set the value to linode-block-storage

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.

4 participants