Skip to content

K8PS-212 cr validations for async cluster type#856

Merged
hors merged 7 commits intomainfrom
K8SPS-212-1
Mar 11, 2025
Merged

K8PS-212 cr validations for async cluster type#856
hors merged 7 commits intomainfrom
K8SPS-212-1

Conversation

@gkech
Copy link
Copy Markdown
Contributor

@gkech gkech commented Mar 6, 2025

K8SPS-212 Powered by Pull Request Badge

CHANGE DESCRIPTION

Problem:
Related PR: #308

Below we have some example of the validations. At the last example we apply the cr with the default values as they are committed in main.

Image: perconalab/percona-server-mysql-operator:K8SPS-212-4

Screenshot 2025-03-07 at 6 12 16 PM

Cause:
Short explanation of the root cause of the issue if applicable.

Solution:
Short explanation of the solution we are providing with this PR.

CHECKLIST

Jira

  • Is the Jira ticket created and referenced properly?
  • Does the Jira ticket have the proper statuses for documentation (Needs Doc) and QA (Needs QA)?
  • Does the Jira ticket link to the proper milestone (Fix Version field)?

Tests

  • Is an E2E test/test case added for the new feature/change?
  • Are unit tests added where appropriate?

Config/Logging/Testability

  • Are all needed new/changed options added to default YAML files?
  • Are all needed new/changed options added to the Helm Chart?
  • Did we add proper logging messages for operator actions?
  • Did we ensure compatibility with the previous version or cluster upgrade process?
  • Does the change support oldest and newest supported PS version?
  • Does the change support oldest and newest supported Kubernetes version?

@pull-request-size pull-request-size bot added the size/M 30-99 lines label Mar 6, 2025
@pull-request-size pull-request-size bot added size/L 100-499 lines and removed size/M 30-99 lines labels Mar 7, 2025
@gkech gkech marked this pull request as ready for review March 7, 2025 16:12
@gkech gkech changed the title K8PS-212 Basic validations on spec K8PS-212 cr validations for async cluster type Mar 7, 2025
@nmarukovich
Copy link
Copy Markdown
Contributor

nmarukovich commented Mar 7, 2025

Maybe it also makes sense to change in our CR orchestrator.enabled true -> false as we use group replication.

And also I have a question maybe for @egegunes @hors I don't remember why we use GR in cr.yaml, but if the value is empty we use Async
Should we use GR in this case too?

if len(cr.Spec.MySQL.ClusterType) == 0 {
		cr.Spec.MySQL.ClusterType = ClusterTypeAsync
	}

@gkech
Copy link
Copy Markdown
Contributor Author

gkech commented Mar 10, 2025

Maybe it also makes sense to change in our CR orchestrator.enabled true -> false as we use group replication.

And also I have a question maybe for @egegunes @hors I don't remember why we use GR in cr.yaml, but if the value is empty we use Async Should we use GR in this case too?

if len(cr.Spec.MySQL.ClusterType) == 0 {
		cr.Spec.MySQL.ClusterType = ClusterTypeAsync
	}

addressed ✅

Comment on lines +47 to +49
// +kubebuilder:validation:XValidation:rule="!(self.mysql.clusterType == 'async') || self.unsafeFlags.orchestrator || self.orchestrator.enabled",message="Invalid configuration: When 'mysql.clusterType' is set to 'async', 'orchestrator.enabled' must be true unless 'unsafeFlags.orchestrator' is enabled"
// +kubebuilder:validation:XValidation:rule="!(self.mysql.clusterType == 'async') || self.unsafeFlags.proxy || self.proxy.haproxy.enabled",message="Invalid configuration: When 'mysql.clusterType' is set to 'async', 'proxy.haproxy.enabled' must be true unless 'unsafeFlags.proxy' is enabled"
// +kubebuilder:validation:XValidation:rule="!(self.mysql.clusterType == 'async') || self.proxy.router == null || !has(self.proxy.router.enabled) || !self.proxy.router.enabled",message="Invalid configuration: When 'mysql.clusterType' is set to 'async', 'proxy.router.enabled' must be disabled"
Copy link
Copy Markdown
Contributor Author

@gkech gkech Mar 11, 2025

Choose a reason for hiding this comment

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

We could introduce more validations e.g. for the cluster sizes, but adding them now means that we will start effecting the unit testing we have like

var _ = Describe("Unsafe configurations", Ordered, func() {
	....

This could also mean that the old validations will become obsolete. We will keep the focus for now only on the cluster type as we initially discussed, and followup with adding more rules here as part of a new ticket that also removes some of the old validations.

@JNKPercona
Copy link
Copy Markdown
Collaborator

Test name Status
version-service passed
async-ignore-annotations passed
auto-config passed
config passed
config-router passed
demand-backup passed
gr-demand-backup passed
gr-demand-backup-haproxy passed
gr-finalizer passed
gr-haproxy passed
gr-ignore-annotations passed
gr-init-deploy passed
gr-one-pod passed
gr-recreate passed
gr-scaling passed
gr-scheduled-backup passed
gr-security-context passed
gr-self-healing passed
gr-tls-cert-manager passed
gr-users passed
haproxy passed
init-deploy passed
limits passed
monitoring passed
one-pod passed
operator-self-healing passed
recreate passed
scaling passed
scheduled-backup passed
service-per-pod passed
sidecars passed
smart-update passed
tls-cert-manager passed
users passed
We run 34 out of 34

commit: b4b31d8
image: perconalab/percona-server-mysql-operator:PR-856-b4b31d89

@hors hors merged commit eb4ab54 into main Mar 11, 2025
16 checks passed
@hors hors deleted the K8SPS-212-1 branch March 11, 2025 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L 100-499 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants