Skip to content

K8SPS-426: use app.kubernetes.io/version label#897

Merged
hors merged 4 commits intomainfrom
K8SPS-426-fix
May 26, 2025
Merged

K8SPS-426: use app.kubernetes.io/version label#897
hors merged 4 commits intomainfrom
K8SPS-426-fix

Conversation

@pooknull
Copy link
Copy Markdown
Contributor

@pooknull pooknull commented May 5, 2025 •
edited by hors
Loading

K8SPS-426 Powered by Pull Request Badge

https://perconadev.atlassian.net/browse/K8SPS-426

DESCRIPTION

This PR replaces mysql.percona.com/version label from #850 with app.kubernetes.io/version.

Also this PR adds the following labels to the crd.yaml:

  labels:
    app.kubernetes.io/component: crd
    app.kubernetes.io/name: percona-server
    app.kubernetes.io/part-of: percona-server-mysql-operator
    app.kubernetes.io/version: v0.10.0

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 May 5, 2025
nmarukovich
nmarukovich previously approved these changes May 5, 2025
View reviewed changes
@pull-request-size pull-request-size bot added size/L 100-499 lines and removed size/M 30-99 lines labels May 5, 2025
egegunes
egegunes requested changes May 6, 2025
View reviewed changes
api/v1alpha1/perconaservermysql_types.go
}
l := naming.Labels()
l[naming.LabelInstance] = cr.Name
l[naming.LabelManagedBy] = "percona-server-operator"
Copy link
Copy Markdown
Contributor

@egegunes egegunes May 6, 2025

Choose a reason for hiding this comment

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

Suggested change
l[naming.LabelManagedBy] = "percona-server-operator"
l[naming.LabelManagedBy] = "percona-server-mysql-operator"

Copy link
Copy Markdown
Contributor Author

@pooknull pooknull May 7, 2025

Choose a reason for hiding this comment

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

This change will cause a lot of changes, including changing a lot of compare files. If we want to follow the recommended labels convention, I would like to do this in another PR as part of this task: https://perconadev.atlassian.net/browse/K8SPS-442

hors
hors requested changes May 22, 2025
View reviewed changes
deploy/crd.yaml
mysql.percona.com/version: v0.10.0
app.kubernetes.io/name: percona-server
app.kubernetes.io/part-of: percona-server
app.kubernetes.io/version: v0.10.0
Copy link
Copy Markdown
Collaborator

@hors hors May 22, 2025

Choose a reason for hiding this comment

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

@pooknull please use the same approach as we did for PSMDBO

  labels:
    app.kubernetes.io/component: crd
    app.kubernetes.io/name: percona-server-mongodb
    app.kubernetes.io/part-of: percona-server-mongodb-operator
    app.kubernetes.io/version: v1.21.0

Copy link
Copy Markdown
Contributor Author

@pooknull pooknull May 26, 2025

Choose a reason for hiding this comment

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

b848709

gkech
gkech reviewed May 26, 2025
View reviewed changes
pkg/naming/naming.go
LabelPartOf = kubernetesPrefix + "part-of"
LabelComponent = kubernetesPrefix + "component"

LabelOperatorVersion = kubernetesPrefix + "version"
Copy link
Copy Markdown
Contributor

@gkech gkech May 26, 2025

Choose a reason for hiding this comment

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

I think that this const is only used in unit tests, maybe because we are adding the version to the crd directory through the generate command. If we don't use it actually somewhere else other than tests maybe it is better to move it in the test file and make it non-exported, or remove it completely and have a constant inside the test function and keep it scoped.

gkech
gkech reviewed May 26, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@gkech gkech left a comment

Choose a reason for hiding this comment

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

when I run make generate manifests VERSION=main I get the following diff, is it expected?

Screenshot 2025-05-26 at 4 39 29 PM

Tried also in main and I don't have this behaviour. Can you confirm?

@hors
Copy link
Copy Markdown
Collaborator

hors commented May 26, 2025

when I run make generate manifests VERSION=main I get the following diff, is it expected?

Screenshot 2025-05-26 at 4 39 29 PM

Tried also in main and I don't have this behaviour. Can you confirm?

Ege fixed it under cfa2781

@hors hors requested review from egegunes, gkech and hors May 26, 2025 14:11
@hors hors merged commit 9981ea8 into main May 26, 2025
15 of 16 checks passed
@hors hors deleted the K8SPS-426-fix branch May 26, 2025 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hors hors hors approved these changes

@egegunes egegunes egegunes approved these changes

@gkech gkech gkech approved these changes

@nmarukovich nmarukovich nmarukovich left review comments

Assignees

No one assigned

Labels

size/L 100-499 lines

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pooknull @hors @egegunes @gkech @nmarukovich