19530 move copy local path based pvs outside of hwn01 or make basemap stateless #1409
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| # Always replace placeholder instances from `base` | ||
| - op: replace | ||
| path: /spec/instances | ||
| value: | ||
| - name: hwn04 | ||
| replicas: 1 | ||
| resources: &resources | ||
| limits: | ||
| memory: 512Gi # # let Postres use the page cache | ||
| cpu: '64' # set as event-api pool size + autovacuum workers + a bit for parallel workers | ||
| # empyrical recommendations below | ||
| # memory: 100Gi | ||
| # cpu: '5' | ||
| requests: | ||
| memory: 22Gi # roughly shared_buffers + pool size * work_mem | ||
| cpu: '2' # if we don't have CPU we can run on potato | ||
| # empyrical recommendations below | ||
| # memory: 7Gi | ||
| # cpu: '1' | ||
| dataVolumeClaimSpec: &storage | ||
| accessModes: | ||
| - ReadWriteOnce | ||
| resources: | ||
| requests: | ||
| storage: 100Gi | ||
| # empyrical recommendations below -- actual storage varies from 2300Gi to 6200Gi | ||
| # storage: 2500Gi | ||
| affinity: | ||
| nodeAffinity: | ||
| requiredDuringSchedulingIgnoredDuringExecution: | ||
| nodeSelectorTerms: | ||
| - matchExpressions: | ||
| - key: kubernetes.io/hostname | ||
| operator: In | ||
| values: | ||
| - hwn04.k8s-01.kontur.io | ||
| - name: hwn03 | ||
| replicas: 1 | ||
| resources: | ||
| <<: *resources | ||
| dataVolumeClaimSpec: | ||
| <<: *storage | ||
| affinity: | ||
| nodeAffinity: | ||
| requiredDuringSchedulingIgnoredDuringExecution: | ||
| nodeSelectorTerms: | ||
| - matchExpressions: | ||
| - key: kubernetes.io/hostname | ||
| operator: In | ||
| values: | ||
| - hwn03.k8s-01.kontur.io | ||
|
|
||
| # Switchover section | ||
| # https://access.crunchydata.com/documentation/postgres-operator/latest/tutorials/cluster-management/administrative-tasks#changing-the-primary | ||
| # https://access.crunchydata.com/documentation/postgres-operator/latest/tutorials/cluster-management/administrative-tasks#targeting-an-instance | ||
| # 1. Requires fully-qualified instance id of new master | ||
| # 2. New master must be in sink with current master (check via `patronictl list`) | ||
| # 3. Its recommended to retain switchover section, to keep track of last desired master both in code and k8s | ||
| - op: add | ||
| path: /spec/patroni/switchover | ||
| value: | ||
| enabled: true | ||
| targetInstance: db-event-api-hwn04-OVERRIDEME | ||
| #type: Failover | ||
| # trigger-switchover annotation triggers actual switchover whenever its updated | ||
| # value is arbitrary, but it's recommended to reference the date of switchover | ||
| # special syntax is used to escape slash with sequence ~1 | ||
| # See https://windsock.io/json-pointer-syntax-in-json-patches-with-kustomize/ | ||
| - op: add | ||
| path: /metadata/annotations/postgres-operator.crunchydata.com~1trigger-switchover | ||
| value: OVERRIDEME |
Empty file.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| # Always replace placeholder instances from `base` | ||
| - op: replace | ||
| path: /spec/instances | ||
| value: | ||
| - name: hwn04 | ||
| replicas: 1 | ||
| resources: &resources | ||
| limits: | ||
| memory: 512Gi # # let Postres use the page cache | ||
| cpu: '64' # set as event-api pool size + autovacuum workers + a bit for parallel workers | ||
| # empyrical recommendations below | ||
| # memory: 100Gi | ||
| # cpu: '5' | ||
| requests: | ||
| memory: 22Gi # roughly shared_buffers + pool size * work_mem | ||
| cpu: '2' # if we don't have CPU we can run on potato | ||
| # empyrical recommendations below | ||
| # memory: 7Gi | ||
| # cpu: '1' | ||
| dataVolumeClaimSpec: &storage | ||
| accessModes: | ||
| - ReadWriteOnce | ||
| resources: | ||
| requests: | ||
| storage: 100Gi | ||
| # empyrical recommendations below -- actual storage varies from 2300Gi to 6200Gi | ||
| # storage: 2500Gi | ||
| affinity: | ||
| nodeAffinity: | ||
| requiredDuringSchedulingIgnoredDuringExecution: | ||
| nodeSelectorTerms: | ||
| - matchExpressions: | ||
| - key: kubernetes.io/hostname | ||
| operator: In | ||
| values: | ||
| - hwn04.k8s-01.kontur.io | ||
| - name: hwn03 | ||
| replicas: 1 | ||
| resources: | ||
| <<: *resources | ||
| dataVolumeClaimSpec: | ||
| <<: *storage | ||
| affinity: | ||
| nodeAffinity: | ||
| requiredDuringSchedulingIgnoredDuringExecution: | ||
| nodeSelectorTerms: | ||
| - matchExpressions: | ||
| - key: kubernetes.io/hostname | ||
| operator: In | ||
| values: | ||
| - hwn03.k8s-01.kontur.io | ||
|
|
||
| # Switchover section | ||
| # https://access.crunchydata.com/documentation/postgres-operator/latest/tutorials/cluster-management/administrative-tasks#changing-the-primary | ||
| # https://access.crunchydata.com/documentation/postgres-operator/latest/tutorials/cluster-management/administrative-tasks#targeting-an-instance | ||
| # 1. Requires fully-qualified instance id of new master | ||
| # 2. New master must be in sink with current master (check via `patronictl list`) | ||
| # 3. Its recommended to retain switchover section, to keep track of last desired master both in code and k8s | ||
| - op: add | ||
| path: /spec/patroni/switchover | ||
| value: | ||
| enabled: true | ||
| targetInstance: db-event-api-hwn04-OVERRIDEME | ||
| #type: Failover | ||
| # trigger-switchover annotation triggers actual switchover whenever its updated | ||
| # value is arbitrary, but it's recommended to reference the date of switchover | ||
| # special syntax is used to escape slash with sequence ~1 | ||
| # See https://windsock.io/json-pointer-syntax-in-json-patches-with-kustomize/ | ||
| - op: add | ||
| path: /metadata/annotations/postgres-operator.crunchydata.com~1trigger-switchover | ||
| value: OVERRIDEME | ||
|
Comment on lines
+53
to
+71
There was a problem hiding this comment. 💡 Codebase verification Action Required: Replace Placeholder Values Before Applying Configuration The switchover configuration contains placeholder values that must be updated before it can be safely applied:
Please replace these placeholders with the appropriate instance identifiers and values to ensure proper functionality. 🔗 Analysis chainLGTM: Switchover configuration with a note on placeholders. The switchover configuration is well-documented with helpful comments explaining the process and requirements. The use of an annotation to trigger the switchover is a good practice for Kubernetes-based operations. However, please note that there are placeholders that need to be replaced before this configuration can be used:
Ensure these are replaced with appropriate values before applying this configuration. To verify the presence of these placeholders, you can run: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check for OVERRIDEME placeholders in the file
grep -n "OVERRIDEME" flux/clusters/k8s-01/event-api-db/overlays/PROD/postgrescluster-instances.yaml
Length of output: 175 |
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Inconsistent Node Affinity Strategy Detected
The addition of a hard-coded hostname (
hwn02.k8s-01.kontur.io) for node affinity in the CronJob introduces an inconsistency with the repository's existing practices, which utilize labels for node selection. This deviation can lead to maintenance challenges and reduced flexibility.Recommendations:
Align with Existing Practices:
Document Rationale:
Consider Flexibility:
preferredDuringSchedulingIgnoredDuringExecutionto ensure the CronJob can run even if the specified node is unavailable.🔗 Analysis chain
Consider using more flexible node selection criteria
The addition of node affinity to the CronJob aligns with the PR objective of moving copy local path based PVs outside of hwn01. However, there are some concerns and suggestions to consider:
hwn02.k8s-01.kontur.io) reduces flexibility and can cause issues if the node is unavailable or renamed.Consider the following recommendations:
Use more flexible node selection criteria, such as node labels or zones, instead of a specific hostname. For example:
Document the reason for this specific node requirement in a comment within the YAML file or in the PR description. This will help future maintainers understand the rationale behind this decision.
Consider implementing a fallback mechanism or using
preferredDuringSchedulingIgnoredDuringExecutioninstead ofrequiredDuringSchedulingIgnoredDuringExecutionto ensure the job can still run if the preferred node is unavailable.To ensure this change doesn't conflict with other scheduling policies, let's check for any existing affinity rules or node selectors:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 76
Script:
Length of output: 1176
Script:
Length of output: 2624