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

Fix clusterresourcequota status deepcopy #11595

Merged
merged 2 commits into from
Oct 28, 2016
Merged

Fix clusterresourcequota status deepcopy #11595

merged 2 commits into from
Oct 28, 2016

Conversation

liggitt
Copy link
Contributor

@liggitt liggitt commented Oct 26, 2016 •
edited
Loading

fixes #11560
based on #11621

@liggitt liggitt changed the title WIP - debug cluster quota mismatch Fix clusterresourcequota status deepcopy Oct 27, 2016
@liggitt
Copy link
Contributor Author

liggitt commented Oct 27, 2016

@smarterclayton @deads2k PTAL

@liggitt
Copy link
Contributor Author

liggitt commented Oct 27, 2016

[test]

@smarterclayton
Copy link
Contributor

LGTM

liggitt
liggitt commented Oct 27, 2016
View reviewed changes
pkg/quota/api/types.go
@@ -115,6 +116,26 @@ func (o *ResourceQuotasStatusByNamespace) OrderedKeys() *list.List {
return o.orderedMap.OrderedKeys()
}

func init() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

now that we were actually copying the internal fields, semantic deepequal choked on comparing different unexported fields. we need to tell it how to compare our custom type. not sure where to hook this (AddKnownTypes? init() here?)

deads2k
deads2k reviewed Oct 27, 2016
View reviewed changes
pkg/quota/api/zz_generated.deepcopy.go
@@ -5,11 +5,12 @@
package api

import (
reflect "reflect"
Copy link
Contributor

Choose a reason for hiding this comment

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

this is what the generator produces now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed up

@deads2k deads2k mentioned this pull request Oct 27, 2016
Closed
@deads2k
Copy link
Contributor

deads2k commented Oct 27, 2016

just the question about deepcopy generation not conflicting.

lgtm

@liggitt
Copy link
Contributor Author

liggitt commented Oct 27, 2016

will pick to 1.3.x/3.3.x once tests are green

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to c37bec0

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10758/) (Base Commit: 0403599)

@liggitt
Copy link
Contributor Author

liggitt commented Oct 28, 2016 •
edited
Loading

green tests
[merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 28, 2016 •
edited
Loading

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10758/) (Image: devenv-rhel7_5263)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to c37bec0

@openshift-bot openshift-bot merged commit 56df948 into openshift:master Oct 28, 2016
@liggitt liggitt deleted the cluster-quota-mismatch branch November 4, 2016 19:09
@liggitt liggitt mentioned this pull request Nov 29, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deads2k deads2k deads2k left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@liggitt @smarterclayton @deads2k @openshift-bot