Using ServiceIPs instead of DNS names in the NetworkPolicy Probes + adding Interface decoupling #102354
Conversation
|
@jayunit100: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. 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. |
|
@jayunit100: This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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. |
k8s-ci-robot
added
area/test
sig/network
approved
sig/testing
cncf-cla: yes
and removed
do-not-merge/needs-sig
labels
|
/test pull-kubernetes-e2e-ubuntu-gce-network-policies |
|
@danwinship just a wip but i think it does the thing you want |
jayunit100
changed the title
|
/retest |
| for _, ns := range model.Namespaces { | ||
| for _, pod := range ns.Pods { | ||
| service := pod.Service() | ||
| kubeService, err := f.ClientSet.CoreV1().Services(pod.Namespace).Get(context.TODO(), service.Name, metav1.GetOptions{}) |
aojea
Jun 15, 2021
Member
why do we have to get the service again?
Services are synchronous and clusterip inmutable
why do we have to get the service again?
Services are synchronous and clusterip inmutable
jayunit100
Jun 15, 2021
Author
Member
pod.Service() is part of the model, which doesn't gaurantee anything about ordering around Service.Create . ill update the comments here to clarify .
pod.Service() is part of the model, which doesn't gaurantee anything about ordering around Service.Create . ill update the comments here to clarify .
jayunit100
Jun 16, 2021
Author
Member
#102919 <-- filed an issue here .... i think the broader question your asking is "why isn't the model stateful" which is i think valid, but we should think a little more deeply about it before adding a one-off cache for service IPs.
As is, we just don't have any assumptions in the framework that model data of any sort, including Service ClusterIPs, needs to be cached.
#102919 <-- filed an issue here .... i think the broader question your asking is "why isn't the model stateful" which is i think valid, but we should think a little more deeply about it before adding a one-off cache for service IPs.
As is, we just don't have any assumptions in the framework that model data of any sort, including Service ClusterIPs, needs to be cached.
aojea
Jun 16, 2021
Member
scratch this comment, I didn't understand how this worked, the service returned is a local object not one from the API server
scratch this comment, I didn't understand how this worked, the service returned is a local object not one from the API server
jayunit100
Jun 16, 2021
Author
Member
either way i think youve touched on something , so ill keep the other issue open
either way i think youve touched on something , so ill keep the other issue open
jayunit100
force-pushed the
jayunit100:netpol_prober
branch
3 times, most recently
from
484202c
to
80af7c9
|
/retest |
jayunit100
force-pushed the
jayunit100:netpol_prober
branch
2 times, most recently
from
8b2dec5
to
c093e8d
|
/retest |
CoreDNS when verifying network policys Update test/e2e/network/netpol/probe.go Co-authored-by: Antonio Ojea <[email protected]> Add deafultNS to use service probe
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jayunit100 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/tide merge-method-none |
|
/tide merge-method-rebase |
|
all set here @aojea ? |
|
/retest |
1 similar comment
|
/retest |
Co-authored-by: Antonio Ojea <[email protected]>
|
/retest |
|
@jayunit100: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
fixes #102286
This addresses removal of network policies dependence on DNS... just a quick hack to make sure it works
will clean up later