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

feat: add possibility to change update strategy #68

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

okaufmann
Copy link
Contributor

Changes in this pull request:

  • Introduce a configuration option to customize the update strategy. This enhancement is particularly beneficial for scenarios involving ReadWriteOnce (RWO) PersistentVolumeClaims (PVCs), where a PVC can be mounted by only one Pod at a time.

@okaufmann okaufmann requested a review from JC5 as a code owner February 5, 2024 20:46
@okaufmann okaufmann force-pushed the add-update-strategy branch 2 times, most recently from 4b4a128 to 9581df7 Compare February 5, 2024 20:55
@JC5
Copy link
Member

JC5 commented Feb 6, 2024

Nice. Could you pick up the lint feedback as well?

@okaufmann okaufmann force-pushed the add-update-strategy branch from 9581df7 to 5cccfc6 Compare February 7, 2024 22:32
@okaufmann
Copy link
Contributor Author

I've set the new version to 1.8.0 as this is a new feature and I guess you are following Semver. If this is not what you want, let me know.

@JC5
Copy link
Member

JC5 commented Feb 8, 2024

That's a great idea. And now for the million dollar question: did you test it, and does it work as expected?

@okaufmann
Copy link
Contributor Author

@JC5 Yes, I run some tests locally and in my Cluster:

cd charts/firefly-iii

helm template test . -f values.yaml

This generated (among the other templates) this Deployment Manifest:

# Source: firefly-iii/templates/deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: test-firefly-iii
  labels:
    helm.sh/chart: firefly-iii-1.8.0
    app.kubernetes.io/name: firefly-iii
    app.kubernetes.io/instance: test
    app.kubernetes.io/managed-by: Helm
spec:
  replicas: 1
  strategy:
    type: RollingUpdate
  # ......

After setting deploymentStrategyType to, and generate the manifest again, I've got this expected output:

# Source: firefly-iii/templates/deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: test-firefly-iii
  labels:
    helm.sh/chart: firefly-iii-1.8.0
    app.kubernetes.io/name: firefly-iii
    app.kubernetes.io/instance: test
    app.kubernetes.io/managed-by: Helm
spec:
  replicas: 1
  strategy:
    type: Recreate
  # ......

I've also updated my Deployment directly on my Cluster and checked it was valid, and it was:
CleanShot 2024-02-14 at 23 58 33

I've removed the additionalrollingUpdate config, which is still visible in the Screenshot (Cannot be there when strategy was set to Recreate). Furthermore, I think when Helm is generating the new Manifests, this config won't be generated and removed automatically on a Chart update after updating deploymentStrategyType in the values.

@JC5 JC5 merged commit 41cf65b into firefly-iii:main Feb 15, 2024
3 checks passed
@JC5
Copy link
Member

JC5 commented Feb 15, 2024

Thanks, merged!

@okaufmann okaufmann deleted the add-update-strategy branch February 15, 2024 10:57
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.

2 participants