K8SPXC-1230 add labels to k8s objects created by operator #1565
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…ator into K8SPXC-1230_add_labels
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
nmarukovich
requested review from
hors,
egegunes,
inelpandzic and
pooknull
as code owners
| @@ -8,7 +8,17 @@ import ( | |||
| ) | |||
|
|
|||
| // NewPVC returns the list of PersistentVolumeClaims for the backups | |||
| func NewPVC(cr *api.PerconaXtraDBClusterBackup) *corev1.PersistentVolumeClaim { | |||
| func NewPVC(cr *api.PerconaXtraDBClusterBackup, cluster *api.PerconaXtraDBCluster) *corev1.PersistentVolumeClaim { | |||
| // Copy from the original labels to the backup labels | |||
There was a problem hiding this comment.
Instead of coping these, there is this function k8s.io/apimachinery/pkg/labels.Merge() that will merge two sets of labels, maybe you can use it.
| @@ -293,6 +293,12 @@ func (r *ReconcilePerconaXtraDBCluster) deletePITR(cr *api.PerconaXtraDBCluster) | |||
| ObjectMeta: metav1.ObjectMeta{ | |||
| Name: deployment.GetBinlogCollectorDeploymentName(cr), | |||
| Namespace: cr.Namespace, | |||
| Labels: map[string]string{ | |||
| "app.kubernetes.io/name": "percona-xtradb-cluster", | |||
There was a problem hiding this comment.
Wdyt that we use this opportunity and refactor this a bit so we don't repeat these strings dozens of times across the repo? To have some constants and reuse them, maybe a helper func in some labels.go file that will add these common labels.
There was a problem hiding this comment.
I checked the code. I don't know. Does it really make sense to create separate utils file to store their only one label?
As I can see the code will become more implicit, more lines of the code and we don't really win something.
What do you think? I'm ok to add it in terms of coding practice, but I really do not see much motivations for it.
There was a problem hiding this comment.
What I meant was that instead of having for example "app.kubernetes.io/name": "percona-xtradb-cluster", all over the place, it will be set only in one file and then just invoke a function that will set the labels.
So the idea is that we have these common labels defined in a single place and then just reuse them where they are needed.
nmarukovich
changed the title
commit: 5c0e1da |
CHANGE DESCRIPTION
Problem:
Short explanation of the problem.
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
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability