Skip to content

Conversation

@runcom
Copy link
Member

@runcom runcom commented Apr 7, 2019

Signed-off-by: Antonio Murdaca [email protected]

- What I did

added cache sync for the other listers we are using in the operator

- How to verify it

CI

- Description for the changelog

@openshift-ci-robot openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 7, 2019
@kikisdeliveryservice
Copy link
Contributor

MCO has error:
E0408 11:13:16.236440 1 reflector.go:134] github.com/openshift/machine-config-operator/pkg/generated/informers/externalversions/factory.go:101: Failed to list *v1.ControllerConfig: the server could not find the requested resource (get controllerconfigs.machineconfiguration.openshift.io)

is that this pr? or?

@runcom
Copy link
Member Author

runcom commented Apr 8, 2019

I believe that's normal since the config operator fails:

items: [
{
apiVersion: "config.openshift.io/v1",
kind: "ClusterOperator",
metadata: {
creationTimestamp: "2019-04-08T10:51:13Z",
generation: 1,
name: "authentication",
resourceVersion: "12577",
selfLink: "/apis/config.openshift.io/v1/clusteroperators/authentication",
uid: "3a112048-59ec-11e9-989d-0a0aad7aab7e"
},
spec: { },
status: {
conditions: [
{
lastTransitionTime: "2019-04-08T10:51:13Z",
message: "Failing: error checking payload readiness: unable to check route health: failed to GET route: EOF",
reason: "Failing",
status: "True",
type: "Failing"
},

nothing is progressing afaict

@kikisdeliveryservice
Copy link
Contributor

I think the 3 informer commits should be squashed here just to tighten up the commits a bit.

@runcom
Copy link
Member Author

runcom commented Apr 9, 2019

I think the 3 informer commits should be squashed here just to tighten up the commits a bit.

They are 3 different units

  • fix a bug
  • refractor a loop
  • refactor client usages

If I'll look at a single commit mixing those together in the future I would feel bad lol

@runcom
Copy link
Member Author

runcom commented Apr 9, 2019

BTW, I've removed the last commit since it's not in #610 🎉

@LorbusChris
Copy link
Contributor

lgtm pending CI success!

@LorbusChris
Copy link
Contributor

LorbusChris commented Apr 9, 2019

aws: bootstrap complete timed out

/retest

@runcom
Copy link
Member Author

runcom commented Apr 9, 2019

I'm starting to think something is broken in this PR as tests aren't passing at all. Will check again later today but I'm pretty sure other PRs are passing tests just fine.

@kikisdeliveryservice
Copy link
Contributor

seeing in e2e-aws/artifacts/e2e-aws/bootstrap/openshift.service:
Apr 09 13:36:25 ip-10-0-10-57 openshift.sh[1346]: error: unable to recognize "./99_openshift-machineconfig_master.yaml": no matches for kind "MachineConfig" in version "machineconfiguration.openshift.io/v1

@runcom
Copy link
Member Author

runcom commented Apr 11, 2019

ok I've finally understood what's happening here! (basically MC,MCP,CC aren't even a thing by the time we try to sync informers cause we haven't installed the CRDs...). I'll rework this out a bit.

@runcom
Copy link
Member Author

runcom commented Apr 11, 2019

Should be fixed now

@runcom
Copy link
Member Author

runcom commented Apr 11, 2019

/retest

@runcom
Copy link
Member Author

runcom commented Apr 11, 2019

oh finally this turned green and the others are going to as well

@kikisdeliveryservice
Copy link
Contributor

/lgtm

@cgwalters
Copy link
Member

Nice work on this, love the split-up commits!

@runcom
Copy link
Member Author

runcom commented Apr 12, 2019

/lgtm

wut, it ignored @kikisdeliveryservice lgtm 🤔

@kikisdeliveryservice
Copy link
Contributor

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kikisdeliveryservice, 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:
  • OWNERS [kikisdeliveryservice,runcom]

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

@kikisdeliveryservice
Copy link
Contributor

@runcom that's really weird!!!

@runcom
Copy link
Member Author

runcom commented Apr 13, 2019

/retest

@openshift-merge-robot openshift-merge-robot merged commit 8a89a61 into openshift:master Apr 13, 2019
@runcom runcom deleted the cachesync-optr branch April 13, 2019 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants