Skip to content

Conversation

@benjaminjb
Copy link
Contributor

@benjaminjb benjaminjb commented Nov 1, 2022

  • Have you added an explanation of what your changes do and why you'd like them to be included?
  • Have you updated or added documentation for the change, as applicable?
  • Have you tested your changes on all related environments with successful results, as applicable?
    • Have you added automated tests?

Type of Changes:

  • New feature
  • Bug fix
  • Documentation
  • Testing enhancement
  • Other

What is the current behavior (link to any open issues here)?

Migration (from Bitnami image + Helm chart) ran into problems and needed manual remediation.

What is the new behavior (if this is a feature change)?

  • Breaking change (fix or feature that would cause existing functionality to change)

This automates the manual remediation for that or similar cases

Other Information:
Issue: [sc-11804]
Issue: [sc-15909]

@benjaminjb benjaminjb force-pushed the migration-assistance branch from 88a6b32 to 75a9477 Compare November 2, 2022 15:50
The postgres-startup container now reports when it finds the installed
PostgreSQL binaries do not match the specified PostgreSQL version.

Some storage providers do not mount the PostgreSQL data volume with
correct ownership or permissions. The postgres-startup container now
prints those attributes of parent directories when it cannot create or
modify a needed file or directory.

Issue: [sc-11804]
Issue: CrunchyData#2870

Co-authored-by: @cbandy
@benjaminjb benjaminjb force-pushed the migration-assistance branch 2 times, most recently from 2f4ad82 to 88e9a62 Compare November 8, 2022 22:59
@benjaminjb
Copy link
Contributor Author

This KUTTL test only works in GKE, but we could alter it (dropping fsGroups) to get it to work in OpenShift. Easy enough to add additional make targets to our CI for GKE/Openshift, but having separate but parallel additional testing opens up some maintainability issues.

@benjaminjb benjaminjb force-pushed the migration-assistance branch from 88e9a62 to 0728a52 Compare November 9, 2022 02:24
delete:
- apiVersion: postgres-operator.crunchydata.com/v1beta1
kind: PostgresCluster
name: cluster-migrate
Copy link
Member

Choose a reason for hiding this comment

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

🤔 We don't usually delete. Is there a benefit to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This came up as a possible topic around our CI best practice: KUTTL will delete ns (unless set not to), but it doesn't consider deletion of namespace as part of the test, so it doesn't wait for the ns to be gone before starting the next test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That said, this delete was actually doing double-duty:

  1. it's maybe best practice for our CI system
  2. I was deleting this cluster and making sure it was gone before manually deleting the PV. (I'm not manually deleting any more, since we switch the Reclaim policy back to what it was before.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After further discussion, I'm removing the delete for now: this will either need to be a convention we decide on (either always or never deleting), or possibly a change in KUTTL, which could be changed to wait for the ns to delete before marking the test as done.

PostgreSQL won't to start unless it owns the data directory. Kubernetes
sets the group according to fsGroup but not the owner.

The postgres-startup container now recreates the data directory to give
it a new owner when permissions are sufficient to do so. It now raises
an error when the owner is incorrect and cannot be changed.

Issue: [sc-15909]
See: https://docs.k8s.io/tasks/configure-pod-container/security-context/

Co-authored-by: @cbandy
@benjaminjb benjaminjb force-pushed the migration-assistance branch 2 times, most recently from 1763f17 to cd187b7 Compare November 11, 2022 15:39
@benjaminjb benjaminjb merged commit 80dfa39 into CrunchyData:master Nov 11, 2022
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