Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[OADP-458] Bring back component: velero label #638

Merged
merged 2 commits into from
Apr 18, 2022
Merged

[OADP-458] Bring back component: velero label #638

merged 2 commits into from
Apr 18, 2022

Conversation

kaovilai
Copy link
Member

@kaovilai kaovilai commented Apr 13, 2022 •
edited by openshift-ci bot
Loading

fix OADP-458

@JaydipGabani
Copy link
Contributor

JaydipGabani commented Apr 13, 2022 •
edited
Loading

@kaovilai I am a little confused so bear with me, the label we are looking for is component: velero which I don't see in these changes, unless that is somehow a default label that now is going to be present. Would you be able to post velero pod yaml in your PR here to display that the desired label is now present after your changes?

kaovilai
kaovilai commented Apr 13, 2022
View reviewed changes
controllers/velero.go
@@ -392,6 +392,7 @@ func (r *DPAReconciler) buildVeleroDeployment(veleroDeployment *appsv1.Deploymen
)
veleroDeployment.TypeMeta = installDeployment.TypeMeta
veleroDeployment.Spec = installDeployment.Spec
veleroDeployment.Labels = installDeployment.Labels
Copy link
Member Author

Choose a reason for hiding this comment

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

@JaydipGabani This adds component: velero to veleroDeployment

kaovilai
kaovilai commented Apr 13, 2022
View reviewed changes
controllers/velero.go
@@ -427,23 +428,16 @@ func removeDuplicateValues(slice []string) []string {
}

func (r *DPAReconciler) customizeVeleroDeployment(dpa *oadpv1alpha1.DataProtectionApplication, veleroDeployment *appsv1.Deployment) error {
veleroDeployment.Labels = r.getAppLabels(dpa)
Copy link
Member Author

Choose a reason for hiding this comment

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

We don't override labels

kaovilai
kaovilai commented Apr 13, 2022
View reviewed changes
controllers/velero.go
},
//append dpa labels
for k, v := range r.getDpaAppLabels(dpa) {
if veleroDeployment.Labels[k] == "" { //saves component: velero labels
Copy link
Member Author

@kaovilai kaovilai Apr 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

We set only if it's not set already
preserves label from https://github.com/openshift/oadp-operator/pull/638/files#r849903351

kaovilai
kaovilai commented Apr 13, 2022
View reviewed changes
controllers/restic.go
@@ -152,6 +152,7 @@ func (r *DPAReconciler) buildResticDaemonset(dpa *oadpv1alpha1.DataProtectionApp
ds.TypeMeta = installDs.TypeMeta
// Update Spec
ds.Spec = installDs.Spec
ds.Labels = installDs.Labels
Copy link
Member Author

@kaovilai kaovilai Apr 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@JaydipGabani this sets label for restic.

@kaovilai
Copy link
Member Author

/retest

@kaovilai
Copy link
Member Author

/retest

1 similar comment
@kaovilai
Copy link
Member Author

/retest

@openshift-ci
Copy link

openshift-ci bot commented Apr 14, 2022

@kaovilai: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@kaovilai kaovilai merged commit a29c162 into openshift:master Apr 18, 2022
@kaovilai kaovilai changed the title Bring back component: velero label [OADP-458] Bring back component: velero label May 6, 2022
kaovilai added a commit to kaovilai/oadp-operator that referenced this pull request May 9, 2022
@kaovilai
Bring back component: velero label (openshift#638)
806a4a9 
* Bring back `component: velero` label, fix OADP-452

* fix component:server on deployment, unit test update
@kaovilai kaovilai mentioned this pull request May 9, 2022
Merged
dymurray pushed a commit that referenced this pull request May 9, 2022
@kaovilai
Cherry-pick 1.0 OADP-452, OADP-458: Bring back component: velero label (
ec3aab2 
#638) (#677)

* Bring back `component: velero` label (#638)

* Bring back `component: velero` label, fix OADP-452

* fix component:server on deployment, unit test update

* label diff for "given valid DPA CR, appropriate velero deployment is build with aws plugin specific specs"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sseago sseago sseago approved these changes

@shubham-pampattiwar shubham-pampattiwar shubham-pampattiwar approved these changes

@pranavgaikwad pranavgaikwad Awaiting requested review from pranavgaikwad

@shawn-hurley shawn-hurley Awaiting requested review from shawn-hurley

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@kaovilai @JaydipGabani @sseago @shubham-pampattiwar