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

Cleanup egress* clientset code #1589

Merged
merged 2 commits into from
Oct 19, 2020
Merged

Cleanup egress* clientset code #1589

merged 2 commits into from
Oct 19, 2020

Conversation

alexanderConstantinescu
Copy link
Collaborator

- What this PR does and why is it needed

This PR takes a step in the direction of cleaning up the code w.r.t all the newly added clientsets from last week. It addresses: #1528 which was a spin-off from the comments given on both #1484 and #1432

In short, it does not do much: simply adds a wrapper around all client sets and allows callers two ways of instantiating the client sets they need, NewOVNClientset (for the wrapper containing them all) or NewKubernetesClientset (only instantiating the Kubernetes client set, which is useful for hybrid overlay which does not need the others).

/cc @danwinship @trozet

- Special notes for reviewers

- How to verify it

- Description for the changelog

@coveralls
Copy link

coveralls commented Aug 6, 2020 •
edited
Loading

Coverage Status

Coverage increased (+0.05%) to 56.452% when pulling 8152cc0 on alexanderConstantinescu:cleanup-egress-crd-infra-code into 6713f6d on ovn-org:master.

danwinship
danwinship reviewed Aug 6, 2020
View reviewed changes
Copy link
Contributor

@danwinship danwinship left a comment

Choose a reason for hiding this comment

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

lgtm

go-controller/pkg/factory/factory.go Show resolved Hide resolved
go-controller/pkg/node/ovn_test.go Show resolved Hide resolved
go-controller/pkg/node/ovn_test.go Show resolved Hide resolved
@alexanderConstantinescu
Copy link
Collaborator Author

/retest

@alexanderConstantinescu
Copy link
Collaborator Author

@danwinship / @trozet , could you have a last look at this and merge, please?

@rcarrillocruz
Copy link
Collaborator

/lgtm

@danwinship
Copy link
Contributor

/retest

@danwinship
Copy link
Contributor

There was a cluster setup failure in the last test run... want to make sure that was just a flake

@alexanderConstantinescu alexanderConstantinescu force-pushed the cleanup-egress-crd-infra-code branch 3 times, most recently from bf3324d to db6806f Compare August 25, 2020 11:43
@alexanderConstantinescu
Copy link
Collaborator Author

The latest test failures are network policy test failures...which seem to be happening across several PRs.

@alexanderConstantinescu
Copy link
Collaborator Author

Again seems to be some hybrid overlay flakes:

OVN Egress Gateway Operations on setting pod gateway annotations 
  reconciles a host networked pod acting as a exgw for another namespace for new pod
  /home/runner/work/ovn-kubernetes/ovn-kubernetes/go-controller/pkg/ovn/egressgw_test.go:280
I0827 13:14:35.344647    9365 watch.go:181] Stopping fake watcher.
I0827 13:14:35.344653    9365 reflector.go:181] Stopping reflector *v1.Endpoints (12h0m0s) from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.344660    9365 watch.go:181] Stopping fake watcher.
I0827 13:14:35.344664    9365 reflector.go:181] Stopping reflector *v1.NetworkPolicy (12h0m0s) from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.344462    9365 reflector.go:181] Stopping reflector *v1.Service (12h0m0s) from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.345357    9365 reflector.go:175] Starting reflector *v1beta1.CustomResourceDefinition (12h0m0s) from k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/factory.go:117
I0827 13:14:35.345375    9365 reflector.go:211] Listing and watching *v1beta1.CustomResourceDefinition from k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/factory.go:117
I0827 13:14:35.445408    9365 shared_informer.go:253] caches populated
I0827 13:14:35.445666    9365 reflector.go:175] Starting reflector *v1.Endpoints (12h0m0s) from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.445677    9365 reflector.go:211] Listing and watching *v1.Endpoints from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.445809    9365 reflector.go:175] Starting reflector *v1.NetworkPolicy (12h0m0s) from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.445816    9365 reflector.go:211] Listing and watching *v1.NetworkPolicy from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.445962    9365 reflector.go:175] Starting reflector *v1.Namespace (12h0m0s) from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.445968    9365 reflector.go:211] Listing and watching *v1.Namespace from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.445977    9365 reflector.go:175] Starting reflector *v1.Service (12h0m0s) from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.445987    9365 reflector.go:211] Listing and watching *v1.Service from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.446097    9365 reflector.go:175] Starting reflector *v1.Node (12h0m0s) from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.446102    9365 reflector.go:211] Listing and watching *v1.Node from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.446184    9365 reflector.go:175] Starting reflector *v1.Pod (12h0m0s) from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.446621    9365 reflector.go:211] Listing and watching *v1.Pod from k8s.io/client-go/informers/factory.go:135
I0827 13:14:35.546083    9365 shared_informer.go:253] caches populated
I0827 13:14:35.546147    9365 shared_informer.go:253] caches populated
I0827 13:14:35.546183    9365 shared_informer.go:253] caches populated
I0827 13:14:35.546227    9365 shared_informer.go:253] caches populated
I0827 13:14:35.546288    9365 shared_informer.go:253] caches populated
I0827 13:14:35.546478    9365 shared_informer.go:253] caches populated
I0827 13:14:35.547045    9365 reflector.go:175] Starting reflector *v1.EgressIP (12h0m0s) from pkg/crd/egressip/v1/apis/informers/externalversions/factory.go:117
I0827 13:14:35.547061    9365 reflector.go:211] Listing and watching *v1.EgressIP from pkg/crd/egressip/v1/apis/informers/externalversions/factory.go:117
I0827 13:14:35.647045    9365 shared_informer.go:253] caches populated
I0827 13:14:35.647213    9365 reflector.go:175] Starting reflector *v1.EgressFirewall (12h0m0s) from pkg/crd/egressfirewall/v1/apis/informers/externalversions/factory.go:117
I0827 13:14:35.647224    9365 reflector.go:211] Listing and watching *v1.EgressFirewall from pkg/crd/egressfirewall/v1/apis/informers/externalversions/factory.go:117
I0827 13:14:35.747282    9365 shared_informer.go:253] caches populated
I0827 13:14:35.747369    9365 ovn.go:873] Bootstrapping existing namespaces and cleaning stale namespaces took 24.5µs
I0827 13:14:35.747432    9365 egressgw.go:24] External gateway pod: gwPod, detected for namespace(s) namespace1
I0827 13:14:35.747651    9365 ovn.go:589] Bootstrapping existing pods and cleaning stale pods took 269.502µs
I0827 13:14:35.747838    9365 kube.go:63] Setting annotations map[k8s.ovn.org/pod-networks:{"default":{"ip_addresses":["10.128.1.3/24"],"mac_address":"0a:58:0a:80:01:03","gateway_ips":["10.128.1.1"],"ip_address":"10.128.1.3/24","gateway_ip":"10.128.1.1"}}] on pod namespace1/myPod
F0827 13:14:35.748111    9365 exec.go:168] Unexpected command: /fake-bin/ovn-nbctl --timeout=15 --may-exist --policy=src-ip --ecmp-symmetric-reply lr-route-add GR_node1 10.128.1.3/32 9.0.0.1

Executed commands do not match expected commands!
[00]   /fake-bin/ovn-nbctl --timeout=15 --data=bare --no-heading --columns=name find logical_switch_port external_ids:pod=true
[01]   /fake-bin/ovn-nbctl --timeout=15 --if-exists remove port_group mcastPortGroupDeny ports 8a86f6d8-7972-4253-b0bd-ddbef66e9303 -- add port_group mcastPortGroupDeny ports 8a86f6d8-7972-4253-b0bd-ddbef66e9303
FAIL	github.com/ovn-org/ovn-kubernetes/go-controller/pkg/ovn	9.219s
FAIL
make: *** [check] Error 1

@alexanderConstantinescu alexanderConstantinescu force-pushed the cleanup-egress-crd-infra-code branch 2 times, most recently from e5edee0 to 7f68e57 Compare August 27, 2020 14:52
@JacobTanenbaum
Copy link
Contributor

/lgtm

@alexanderConstantinescu
Ignore all yaml files in dist/yaml
c9fd676 
Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
@alexanderConstantinescu
Adding OVNClient wrapper around all required clientsets
8152cc0 
Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
@alexanderConstantinescu
Copy link
Collaborator Author

@danwinship : could you please have a look at this? Sorry to be pinging you all over the place, but you're the only one who already had a look at this and can merge it.

@danwinship
Copy link
Contributor

lgtm, sorry for dropping this

@danwinship danwinship merged commit bced278 into ovn-org:master Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danwinship danwinship danwinship left review comments

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

Successfully merging this pull request may close these issues.
5 participants
@alexanderConstantinescu @coveralls @rcarrillocruz @danwinship @JacobTanenbaum