Skip to content

Bug 1703234: pkg/operator: ignore CM changes that we don't care about#813

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
runcom:log-change-optr
Jun 5, 2019
Merged

Bug 1703234: pkg/operator: ignore CM changes that we don't care about#813
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
runcom:log-change-optr

Conversation

@runcom
Copy link
Copy Markdown
Member

@runcom runcom commented Jun 4, 2019 •
edited
Loading

Triggering a sync on the following CMs chnages causes a wasteful sync (every 14s):

NAME                             DATA   AGE
machine-config                   0      27m
machine-config-controller        0      27m

Those cms seem to be bound to operator and controller leases. Since we
don't need to listen on them for our sync, just ignore them.

I'm following up with someone who understands more than me on this but it shouldn't be an issue to just ignore those.

Signed-off-by: Antonio Murdaca runcom@linux.com

@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 4, 2019
@runcom runcom force-pushed the log-change-optr branch from d3698d2 to b151730 Compare June 4, 2019 14:31
@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 4, 2019

/retest

@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 4, 2019

/hold

this is quite spammy actually - holding till I at least figure stuff in my test cluster - we can drop this to glog v2 maybe.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 4, 2019
@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 4, 2019

ok, somehow we have a ConfigMap resyncinc every 14s for no good reason - trying to understand why now.

@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 4, 2019 •
edited
Loading 

I0604 16:04:52.546178       1 operator.go:257] resyncing operator because "openshift-machine-config-operator/machine-config-controller" *v1.ConfigMap changed
I0604 16:04:59.840257       1 operator.go:257] resyncing operator because "openshift-machine-config-operator/machine-config" *v1.ConfigMap changed

@runcom runcom force-pushed the log-change-optr branch from b151730 to e641349 Compare June 4, 2019 16:33
@runcom runcom changed the title pkg/operator: log object change from informers Bug 1703234: pkg/operator: ignore CM changes that we don't care about Jun 4, 2019
@runcom runcom force-pushed the log-change-optr branch from e641349 to f68d30a Compare June 4, 2019 16:37
@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 4, 2019

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 4, 2019
@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 4, 2019

/hold

We need to wholly ignore Config maps based on the leader annotation (got word from Clayton). I'll change this later today.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 4, 2019
@runcom runcom force-pushed the log-change-optr branch 2 times, most recently from 1c06ae3 to 738bc47 Compare June 4, 2019 22:03
@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 4, 2019 •
edited
Loading

/hold cancel

fully good to go now! and tested that we're indeed idle as we should be

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 4, 2019
@runcom
pkg/operator: ignore CM changes that we don't care about
21c59b2 
Triggering a sync on the following CMs causes a wasteful sync:

NAME                             DATA   AGE
machine-config                   0      27m
machine-config-controller        0      27m

Those cms seem to be bound to operator and controller leases. Since we
don't need to listen on them for our sync, just ignore them.

Signed-off-by: Antonio Murdaca <runcom@linux.com>
@runcom runcom force-pushed the log-change-optr branch from 738bc47 to 21c59b2 Compare June 4, 2019 22:09
@runcom runcom mentioned this pull request Jun 4, 2019
Closed
@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 5, 2019

/retest

3 similar comments
@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 5, 2019

/retest

@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 5, 2019

/retest

@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 5, 2019

/retest

@runcom runcom mentioned this pull request Jun 5, 2019
Closed
@cgwalters
Copy link
Copy Markdown
Member

cgwalters commented Jun 5, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 5, 2019
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented Jun 5, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, runcom

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented Jun 5, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 5, 2019

/retest

@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented Jun 5, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented Jun 5, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 5, 2019

leaning towards skipping that test..........

@kikisdeliveryservice
Copy link
Copy Markdown
Contributor

kikisdeliveryservice commented Jun 5, 2019

leaning towards skipping that test..........

the etcd quorum loss one? looking at the runs it hasn't had a successful one on this pr at all and only 2 successful runs since Monday... yikes

@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 5, 2019

the etcd quorum loss one? looking at the runs it hasn't had a successful one on this pr at all and only 2 successful runs since Monday... yikes

yeah, it's known to be highly flake :(

@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 5, 2019

/retest

1 similar comment
@runcom
Copy link
Copy Markdown
Member Author

runcom commented Jun 5, 2019

/retest

@openshift-merge-robot openshift-merge-robot merged commit faad525 into openshift:master Jun 5, 2019
@runcom runcom deleted the log-change-optr branch June 5, 2019 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cgwalters cgwalters Awaiting requested review from cgwalters

@LorbusChris LorbusChris Awaiting requested review from LorbusChris

Assignees

@cgwalters cgwalters

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@runcom @cgwalters @openshift-ci-robot @openshift-bot @kikisdeliveryservice @openshift-merge-robot