Skip to content

Conversation

@nirroz93
Copy link
Contributor

@nirroz93 nirroz93 commented Mar 21, 2024


Add persistentVolumeClaimRetentionPolicy available since k8s 1.23 (alpha) and 1.27 (beta). With logs, this makes a lot of sense (with remote logging only running task log is important, and on scale down pvc should be removed)

@eladkal eladkal added this to the Airflow Helm Chart 1.14.0 milestone Mar 22, 2024
@eladkal eladkal requested a review from amoghrajesh April 16, 2024 04:57
Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

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

@nirroz93 good work on the PR. Few comments:

  1. Can we add this also to chart/files/pod-template-file.kubernetes-helm-yaml for K8sExecutor?
  2. Can we add unit tests foe this under helm_tests? Check similar tests.

persistence:
# Enable persistent volumes
enabled: true
# persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled\removed
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
# persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled\removed
# persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled/removed

persistence:
# Enable persistent volumes
enabled: true
# persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled\removed
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled\removed
# persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled/removed

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Jun 10, 2024
@romsharon98
Copy link
Contributor

@nirroz93 good work on the PR. Few comments:

  1. Can we add this also to chart/files/pod-template-file.kubernetes-helm-yaml for K8sExecutor?
  2. Can we add unit tests foe this under helm_tests? Check similar tests.

@nirroz93 good work on the PR. Few comments:

  1. Can we add this also to chart/files/pod-template-file.kubernetes-helm-yaml for K8sExecutor?
  2. Can we add unit tests foe this under helm_tests? Check similar tests.

no need to add this to chart/files/pod-template-file.kubernetes-helm-yaml because this feature belongs only to statefulset.

@amoghrajesh
Copy link
Contributor

@nirroz93 are you actively working on this?

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

Labels

area:helm-chart Airflow Helm Chart stale Stale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants