| *** hongbin has joined #openstack-kuryr | 00:18 | |
| *** celebdor has quit IRC | 01:37 | |
| *** rh-jelabarre has quit IRC | 02:25 | |
| *** kmadac4 has joined #openstack-kuryr | 03:01 | |
| *** kmadac3 has quit IRC | 03:04 | |
| *** hongbin has quit IRC | 03:28 | |
| *** janki has joined #openstack-kuryr | 04:41 | |
| *** premsankar has quit IRC | 05:11 | |
| *** gcheresh has joined #openstack-kuryr | 06:08 | |
| *** gcheresh has quit IRC | 06:50 | |
| *** gcheresh has joined #openstack-kuryr | 06:56 | |
| *** ccamposr has joined #openstack-kuryr | 07:00 | |
| *** maysams has quit IRC | 07:14 | |
| *** pcaruana has joined #openstack-kuryr | 07:20 | |
| *** yboaron_ has joined #openstack-kuryr | 07:22 | |
| *** celebdor has joined #openstack-kuryr | 08:04 | |
| aperevalov_ | Hi, I told about definition of VF used by SRIOVDP, now it's more clear we need to obtain pciaddress, and seems to be it's possible to do by obtaining pod Resources | 08:11 |
|---|---|---|
| aperevalov_ | https://godoc.org/k8s.io/kubernetes/pkg/kubelet/apis/podresources | 08:11 |
| aperevalov_ | How do you think is K8sClient proper place to implement this request? | 08:12 |
| *** maysams has joined #openstack-kuryr | 08:18 | |
| dulek | aperevalov_: Is it a simple get? Then you have everything you need there. | 09:04 |
| aperevalov_ | dulek, it's grpc, but yes just get, but on the other dedicated socket unix:/var/lib/kubelet/pod-resources/kubelet.sock. We think it's missed in public API. | 09:09 |
| *** oanson has quit IRC | 09:10 | |
| *** snapiri has quit IRC | 09:10 | |
| dulek | aperevalov_: Ah, okay. If it's grcp, can you create a new client class? | 09:12 |
| dulek | aperevalov_: I'm asking because we plan to use grcp for kuryr-cni->kuryr-daemon communication in the future. | 09:12 |
| dulek | aperevalov_: So it'll be useful to have it in another entity. | 09:13 |
| dulek | aperevalov_: How stable are those GRCP API's? | 09:13 |
| aperevalov_ | it appeared in k8s 1.13 and by default it's disabled, API in v1alpha1 version. So it's not stable in time yet. | 09:14 |
| aperevalov_ | it mostly used by prometheus | 09:15 |
| *** snapiri has joined #openstack-kuryr | 09:18 | |
| *** shachar has joined #openstack-kuryr | 09:21 | |
| *** snapiri has quit IRC | 09:22 | |
| openstackgerrit | Merged openstack/kuryr-kubernetes master: Update master for stable/stein https://review.openstack.org/648210 | 09:23 |
| *** oanson has joined #openstack-kuryr | 09:41 | |
| openstackgerrit | Daniel Mellado proposed openstack/kuryr-kubernetes master: Add ipBlock support to NP https://review.openstack.org/645139 | 09:45 |
| dulek | aperevalov_: Ah, okay then - if it's used by an outside project, then it'll be more-or-less stable. | 10:19 |
| dulek | So it's totally fine you'll use it. :) | 10:19 |
| dulek | dmellado, ltomasbo: I fixed https://review.openstack.org/#/c/648209/ updating the upper-req links in Dockerfiles as well.. | 10:52 |
| ltomasbo | dulek, great! thanks! | 10:52 |
| openstackgerrit | Daniel Mellado proposed openstack/kuryr-kubernetes master: Add support for text ports on Network Policy Spec https://review.openstack.org/643080 | 11:06 |
| *** janki has quit IRC | 11:18 | |
| openstackgerrit | Maysa de Macedo Souza proposed openstack/kuryr-kubernetes master: Add support for text ports on Network Policy Spec https://review.openstack.org/643080 | 11:40 |
| maysams | ltomasbo, dmellado: I've updated it to cover more cases ^ | 11:42 |
| dmellado | maysams: did you see my rebase? | 11:42 |
| dmellado | nope, looks like you didn't xD | 11:43 |
| maysams | dulek: When you have some free time could you run the upstream tests again with my ps applied? | 11:43 |
| dmellado | please get PS 10 and reapply your changes onto it | 11:43 |
| dmellado | maysams: ^^ | 11:43 |
| maysams | dmellado: No, I did not see it yet | 11:43 |
| dmellado | maysams: check PS10 from your patch | 11:43 |
| dulek | maysams: Sure, but it'll take a moment as I'll need to recreate the env. | 11:43 |
| *** janki has joined #openstack-kuryr | 11:43 | |
| dmellado | https://review.openstack.org/#/c/643080/10..11/kuryr_kubernetes/controller/drivers/network_policy.py | 11:44 |
| dmellado | i.e. | 11:44 |
| maysams | dmellado: oh god, I did not see it at all | 11:44 |
| dmellado | maysams: please reapply that as well as your new changes ;) | 11:44 |
| dmellado | and ping me if there's anything that would block you | 11:45 |
| dmellado | thanks! | 11:45 |
| maysams | dmellado: I will do it after lunch. Sorry for that | 11:45 |
| dmellado | no worries! | 11:45 |
| *** rh-jelabarre has joined #openstack-kuryr | 12:01 | |
| dulek | dmellado: Why is https://review.openstack.org/#/c/648209/ not necessary? | 12:06 |
| *** celebdor has quit IRC | 12:41 | |
| *** celebdor has joined #openstack-kuryr | 12:50 | |
| dmellado | dulek: I already created the stable branch and thought this was duplicated, but hold on | 13:18 |
| dmellado | yep | 13:18 |
| dmellado | not necessary IIUC | 13:18 |
| dmellado | https://github.com/openstack/requirements/tree/stable/stein | 13:18 |
| dmellado | requirements has already a stable/stein branch so stable/stein kuryr will be matched against it | 13:19 |
| dmellado | but I missed the dockerfile, would you mind amending a change to that that would only change the dockerfiles? | 13:19 |
| dmellado | thanks! | 13:19 |
| dulek | dmellado: It changes the tox envs to point to stable/stein upper-reqs from openstack/requirements. | 13:27 |
| dulek | https://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints.txt will always point to master. | 13:27 |
| dulek | https://releases.openstack.org/constraints/upper/stein will point to stein. | 13:27 |
| dulek | dmellado: So IMO it doesn't matter if openstack/requirements are branched or not. | 13:29 |
| dulek | maysams: The env is stacking. BTW - you can try to run those yourself actually. | 13:33 |
| dulek | Not sure why I thought of that only now. :P | 13:33 |
| dulek | maysams: It's just `go get k8s.io/test-infra/kubetest && export PATH=$PATH:${HOME}/go/bin && kubetest --extract=${KUBETEST_VERSION} --provider=local --test --test_args="--ginkgo.focus=\[${KUBETEST_FOCUS}\]"` | 13:34 |
| maysams | dulek: really? but this is going to run all the tests right? | 14:04 |
| dulek | maysams: Ah, right, just a sec. | 14:05 |
| dulek | maysams: kubetest --test --test_args='--ginkgo.focus="should allow egress access on one named port \[Feature:NetworkPolicy\]"' --provider=local | 14:05 |
| dulek | maysams: This way you only run one. | 14:05 |
| maysams | nice, thanks dulek! | 14:08 |
| maysams | dmellado: ping | 14:23 |
| dmellado | maysams: pong, in a meeting but shoot and I'll reply asap | 14:25 |
| maysams | dmellado: I was taking a look in your ps, we cannot rebase as is. We need to agree on how to handle the _parse_selectors function | 14:37 |
| maysams | dmellado: note in that function that instead of returning the cidr and namespace of each selected resource, I changed to return the resource itself | 14:40 |
| *** yboaron_ has quit IRC | 14:44 | |
| maysams | dmellado: so we would need to rework you ps or mine. Let me know what do you think | 14:44 |
| maysams | s/you/your | 14:45 |
| maysams | dulek: Are you still going to run the test on your env? Just remembered you have OVN enabled | 14:48 |
| maysams | mine is with Octavia | 14:49 |
| dulek | maysams: Sure, but you'll need to decide which patchset I should take. :D | 14:49 |
| maysams | XD | 14:49 |
| maysams | dulek: the last one here https://review.openstack.org/#/c/643080/ | 14:49 |
| dmellado | maysams: well, I worked that around so I'm returning cidr/namespace only in that case as I'm parsing from the policy spec | 14:58 |
| dmellado | IMHO it'd be cool with that, but feel free to send a PS if you'd prefer to redo anything | 14:59 |
| dmellado | or comment there on mine/your patch and I'll be taking a look | 14:59 |
| dmellado | I still have hours of meetings, sorry xD | 14:59 |
| maysams | dmellado: right, I will leave a few comments regarding that and a few other things | 15:00 |
| maysams | dmellado: Thanks, and sorry for disturbing you | 15:01 |
| dmellado | maysams: no worries! | 15:01 |
| dmellado | feel free to ping me in any case, and I'll reply whenever I can | 15:01 |
| dmellado | I'm fine with async communication xD | 15:01 |
| dmellado | but if there's something you'd rather discuss IRL let me know and we'll do a quick bluejeans | 15:01 |
| *** yboaron_ has joined #openstack-kuryr | 15:05 | |
| dulek | maysams: Mar 28 15:51:36.173: Pod client-a should be able to connect to service svc-server, but was not able to connect. | 15:52 |
| maysams | whaaat | 15:52 |
| dulek | maysams: What should I check? | 15:55 |
| maysams | dulek: would you mind sharing the sg applied on the client pod? | 15:55 |
| dulek | Though it might be easier for you to setup OVN's DevStack. :P | 15:55 |
| dulek | maysams: Hm, okay, so one more run… | 15:56 |
| maysams | dulek: true. Could you share the local.conf with me? | 15:56 |
| dulek | maysams: Were we running the ingress or egress test last time? | 15:57 |
| maysams | egress | 15:57 |
| dulek | Okay, so this one's the same, good. | 15:58 |
| dulek | maysams: https://paste.fedoraproject.org/paste/Up6qLScZlPhgLTmBIscGyg | 15:58 |
| dulek | maysams: Just remember that you need that specific version of networking-ovn. | 15:59 |
| *** yboaron_ has quit IRC | 15:59 | |
| *** yboaron_ has joined #openstack-kuryr | 16:00 | |
| dulek | maysams: Here's the SG on client-a: https://paste.fedoraproject.org/paste/4Cg3JmnhznC1ybNNVKHU7Q | 16:01 |
| dulek | Ingress on 10.1.0.128/26 looks cool, right? | 16:01 |
| dulek | Ah, wait, that's client. :P | 16:02 |
| *** premsankar has joined #openstack-kuryr | 16:03 | |
| maysams | dulek: well, the correct egress rule was created | 16:03 |
| dulek | Yep, let's look deeper: https://paste.fedoraproject.org/paste/R~rdwVw3GqIs8SjgPiRBAQ | 16:04 |
| dulek | This is LB's port. | 16:04 |
| dulek | It allows anything ingress on 80 and 81, which is fine, right? | 16:04 |
| maysams | totally fine | 16:04 |
| maysams | the problem seems to be with the egress default rules | 16:04 |
| maysams | in the client | 16:04 |
| maysams | if you remove them I have a filling it will work | 16:05 |
| dulek | maysams: I don't think it's the cause. | 16:06 |
| dulek | maysams: See the server: https://paste.fedoraproject.org/paste/HdvWYbH7~g3Lxo35o-N8YQ/ | 16:06 |
| dulek | maysams: It's the SG of server port. I'm skipping the default ones. | 16:06 |
| dulek | See? It's not having anything that will allow connections from client-a. | 16:06 |
| *** janki has quit IRC | 16:07 | |
| maysams | which subnet is that 10.1.0.64/26 ? | 16:07 |
| maysams | svc? | 16:07 |
| dulek | Ah, I see, just a second… | 16:07 |
| dulek | maysams: Heh, what if I say neither? :D | 16:08 |
| maysams | dulek: xD which one then? | 16:09 |
| dulek | maysams: Okay, that is set as k8s-pod-subnet. | 16:10 |
| dulek | But I don't think I use it - it's env with subnet-per-namespace, right? | 16:10 |
| dulek | maysams: Pod subnets are at 10.1.0.0/22. | 16:10 |
| dulek | This might be it! | 16:11 |
| dulek | Yes, pod_subnet is set, although it's never used. | 16:11 |
| dulek | maysams: So for some reason pod_subnet CIDR's was taken and set as ingress part here. Now - why? | 16:15 |
| maysams | dulek: When a pod is not selected by a NP, which is the case for the server, we apply the default pod sg | 16:17 |
| dulek | maysams: This one is octavia_pod_access. | 16:18 |
| dulek | ltomasbo: Ping? | 16:19 |
| ltomasbo | dulek, pong | 16:19 |
| dulek | ltomasbo: What's the purpose of octavia_pod_access? | 16:19 |
| dulek | SG | 16:19 |
| ltomasbo | let me remember that... | 16:20 |
| dulek | maysams: You're right, it's one of SG's on [neutron_defaults]pod_security_groups. | 16:21 |
| ltomasbo | I think it was for octavia L2 mode | 16:21 |
| ltomasbo | dulek, ^^ | 16:21 |
| dulek | ltomasbo: Hm, so we're not looking in a correct direction, I guess. | 16:22 |
| maysams | dulek: the problem might be, because there is no ingress rule in the po server allowing traffic from the SVC subnet | 16:22 |
| ltomasbo | dulek, sorry I was not following the conversation... | 16:23 |
| ltomasbo | dulek, https://github.com/openstack/kuryr-kubernetes/blob/master/devstack/plugin.sh#L347-L352 | 16:23 |
| dulek | Oh crap, I'm actually using the L2 mode! | 16:24 |
| dulek | For the OVN stuff. | 16:24 |
| dulek | ltomasbo: We're debugging why on OVN one test of usptream k82 e2e NP tests is failing. | 16:24 |
| ltomasbo | dulek, ok, for OVN it may actually be a bit different | 16:24 |
| ltomasbo | failing as in not blocking traffic? | 16:24 |
| dulek | It's blocking the one that should go through. | 16:25 |
| ltomasbo | or as in, the traffic is not reaching there when it should? | 16:25 |
| ltomasbo | ok, ovn L2 is actually a bit different | 16:25 |
| ltomasbo | as octavia_pod_access is only needed for amphoras | 16:25 |
| dulek | ltomasbo: Everything is fine on client's port, on LB port and there's no correct ingress rule on the server. | 16:25 |
| dulek | server pod port. | 16:25 |
| dulek | ltomasbo: So one bug is that we put wrong CIDR there when running with namespace driver. | 16:26 |
| dulek | ltomasbo: With subnet this: https://github.com/openstack/kuryr-kubernetes/blob/3e3ed9dbb31133b5175ff792a9636603b2df95e1/devstack/plugin.sh#L365 | 16:26 |
| dulek | ltomasbo: Should have CIDR of the subnetpool, isn't it? | 16:26 |
| ltomasbo | dulek, umm... not sure actually... let me think | 16:27 |
| dulek | maysams: I saw your comment, I think it depends if traffic is marked as originating from LB or from the source pod. :P | 16:27 |
| ltomasbo | dulek, problem is, that I think that rule should not be there in case of namespace isolation | 16:30 |
| ltomasbo | if we put all the subnetpool, then we break isolation, right? | 16:31 |
| dulek | ltomasbo: Uhm, fair point… Okay, let's put it aside then. | 16:31 |
| dulek | ltomasbo: It's only opening more and in our case we have too little open. :P | 16:31 |
| ltomasbo | dulek, but yep, we need to think about it... and how we have it working... | 16:32 |
| dulek | ltomasbo, maysams: So what's supposed to open an SG on loadbalancer listeners? | 16:32 |
| ltomasbo | dulek, just to be on track | 16:32 |
| ltomasbo | dulek, you are trying NPs | 16:32 |
| dulek | Because clearly it shouldn't be on default. :D | 16:32 |
| dulek | ltomasbo: Yep! | 16:32 |
| ltomasbo | dulek, with ovn base deployment, right? | 16:32 |
| ltomasbo | and with ovn-provider for lbs? | 16:33 |
| dulek | ltomasbo: Yes. With L2, apparently. | 16:33 |
| dulek | Yes! | 16:33 |
| ltomasbo | ok, and you are trying to curl | 16:33 |
| ltomasbo | podA -> serviceB -> podB | 16:33 |
| maysams | yup | 16:33 |
| ltomasbo | and network policy must allow that traffic? | 16:33 |
| ltomasbo | since there is an ingress policy enabling traffic on podB? | 16:34 |
| dulek | ltomasbo: Uh, apparently yes - that's how test is built. | 16:34 |
| dulek | Let me find it… | 16:34 |
| dulek | (did we simply misunderstood the NP spec?) | 16:34 |
| ltomasbo | can you paste the spec? | 16:35 |
| maysams | how? | 16:35 |
| dulek | ltomasbo: https://github.com/kubernetes/kubernetes/blob/master/test/e2e/network/network_policy.go#L314-L356 | 16:35 |
| dulek | By spec I mean the K8s document describing the feature. | 16:36 |
| dulek | So probably this: https://kubernetes.io/docs/concepts/services-networking/network-policies/ | 16:36 |
| ltomasbo | dulek, good enough! xD\ | 16:36 |
| ltomasbo | dulek, ahh, that is egress! | 16:36 |
| ltomasbo | that is not tested... so probably we are not generating the right SG group... | 16:37 |
| dulek | ltomasbo: Yes, it's egress. And client-a has correct egress rules applied. | 16:37 |
| ltomasbo | on podA | 16:37 |
| ltomasbo | dulek, what about ingress? | 16:38 |
| ltomasbo | on podA? | 16:38 |
| dulek | ltomasbo: Nope, it's fine: https://paste.fedoraproject.org/paste/4Cg3JmnhznC1ybNNVKHU7Q | 16:38 |
| ltomasbo | it is allowed? | 16:38 |
| dulek | ltomasbo: This is podA a.k.a client-a in the test. | 16:38 |
| maysams | ltomasbo: it's not accepting ingress from the svc subnet only po subnet | 16:38 |
| ltomasbo | 10.1.0/128/26 is service CIDR? | 16:39 |
| dulek | ltomasbo: This looks nice: direction='egress', ethertype='IPv4', id='6ad62b4f-f802-4cab-a717-c66e86297787', port_range_max='80', port_range_min='80', protocol='tcp', updated_at='2019-03-28T15:58:51Z' | 16:39 |
| dulek | ltomasbo: Yes. | 16:39 |
| ltomasbo | so, perhaps with ovn-provider, the reply is with destination pod IP | 16:40 |
| ltomasbo | or, you are hitting the bug for the neutron SG rules not being properly managed... | 16:40 |
| ltomasbo | thought that should be on ml2/ovs not ovn (I hope) | 16:41 |
| ltomasbo | with tcpdump, can you check the destination pod receives and reply to traffic? | 16:41 |
| maysams | ltomasbo: Do you think this is correct? This is SG applied on the Server pod https://paste.fedoraproject.org/paste/HdvWYbH7~g3Lxo35o-N8YQ | 16:42 |
| dulek | ltomasbo, maysams: Let's take a step back, okay? | 16:42 |
| ltomasbo | server pod? | 16:42 |
| dulek | ltomasbo: So what we want to do is to do client-a -> LB -> server | 16:42 |
| ltomasbo | dulek, got it! | 16:43 |
| dulek | ltomasbo: client-a, as you can see, has egress everywhere due to default rules, so no problem. | 16:43 |
| dulek | This is client-a https://paste.fedoraproject.org/paste/4Cg3JmnhznC1ybNNVKHU7Q | 16:43 |
| ltomasbo | dulek, I dont see everywhere | 16:44 |
| ltomasbo | but port 80 | 16:44 |
| ltomasbo | (and 53) | 16:44 |
| ltomasbo | ahh, you mean any destination... ok | 16:44 |
| dulek | ltomasbo: And SG rule aba4221a-74d8-403f-9e4a-baed9304501e? | 16:44 |
| ltomasbo | ahh, true | 16:44 |
| dulek | ltomasbo: :) | 16:45 |
| ltomasbo | ok, next | 16:45 |
| ltomasbo | what SGs are on the lbs? | 16:45 |
| dulek | ltomasbo: Now let me just ask - to receive the response it doesn't need a corresponding ingress rule, right? | 16:45 |
| dulek | LB: https://paste.fedoraproject.org/paste/R~rdwVw3GqIs8SjgPiRBAQ | 16:45 |
| dulek | Seems good, correct ingress rules are there. | 16:45 |
| dulek | Right? | 16:46 |
| ltomasbo | yep, allowing port 80 and 81 | 16:46 |
| dulek | ltomasbo: So now this is the server pod: https://paste.fedoraproject.org/paste/HdvWYbH7~g3Lxo35o-N8YQ | 16:46 |
| dulek | ltomasbo: Ah, and just a sec, there's a second rule. | 16:46 |
| dulek | ltomasbo: https://paste.fedoraproject.org/paste/ki-BJxYL92StgynHptgyig/ | 16:47 |
| dulek | So server pod has those two. We can ignore the first one, it's octavia_pod_access, we assume it shouldn't really be there. | 16:48 |
| dulek | ltomasbo: So we're only left with the default there. Which doesn't allow ingress from the outside. | 16:48 |
| dulek | ltomasbo: And client-a and LB has no default rule applied. | 16:48 |
| dulek | Which seems to make sense, isn't it? | 16:48 |
| ltomasbo | umm, ok | 16:49 |
| ltomasbo | what sg is 34faaab2-0fd0-460a-8ecd-027da62cd1bc | 16:49 |
| dulek | ltomasbo: It's pointing to itself. :P | 16:49 |
| ltomasbo | ok, I think I got it now | 16:50 |
| ltomasbo | without namespaces, connectivity is ensure by adding the same sg to all the pods, rights? | 16:50 |
| ltomasbo | now, we are removing those on some pods (for the egress) and not in the others (no ingress) | 16:50 |
| dulek | ltomasbo: Yeaaah, I think so. | 16:50 |
| dulek | ltomasbo: Yes, but hold a second. | 16:50 |
| dulek | ltomasbo, maysams: Now my point is, do we think that our behavior is correct here, as server pod has no ingress NP? | 16:51 |
| ltomasbo | shouldn't we have a default SG being applied? that allows everything? | 16:51 |
| dulek | ltomasbo: Where? | 16:51 |
| maysams | dulek: in the server pod | 16:51 |
| maysams | I think | 16:52 |
| ltomasbo | no, I think the accepted behavior is that, unless a NP is pointing to it, it should be open | 16:52 |
| maysams | right | 16:52 |
| dulek | Ooooh… | 16:52 |
| dulek | That's missing, isn't it? | 16:52 |
| dulek | :D | 16:52 |
| ltomasbo | dulek, but we are creating this SG on devstack, right? | 16:52 |
| *** ccamposr has quit IRC | 16:52 | |
| dulek | ltomasbo: Let me check what's on my env. | 16:52 |
| ltomasbo | it will be as simple as changing it by open ingress for everything, right? | 16:53 |
| ltomasbo | that should not be used for namespace isolation (the default sg) | 16:53 |
| dulek | ltomasbo: Yes, it would be. | 16:53 |
| ltomasbo | and once the NP is applied, it should be removed | 16:53 |
| dulek | ltomasbo: So I don't have any allow-all SG in my env… | 16:53 |
| ltomasbo | dulek, yep, instead of allow all, it seems we had that sg that allows from himself, right? | 16:54 |
| dulek | ltomasbo: Yes, but see, that's the default SG, created by OpenStack, not us. | 16:55 |
| ltomasbo | ahh, and why are we using that one? | 16:55 |
| ltomasbo | probably that is why it is different from openshift-ansible? | 16:55 |
| dulek | ltomasbo: Well, I don't know. :D Looking at DevStack at the moment. | 16:55 |
| dulek | ltomasbo: It is different from openshfit-ansible in what sense? | 16:56 |
| ltomasbo | https://github.com/openshift/openshift-ansible/blob/release-3.11/roles/openshift_openstack/templates/heat_stack.yaml.j2#L459 | 16:57 |
| ltomasbo | dulek, ^^ in the sense we are creating the default sg group and rules | 16:58 |
| dulek | Hey, dmellado, still here? | 16:59 |
| dmellado | hey dulek, in meetings, but still around, what's up? | 16:59 |
| dulek | dmellado: In the initial NP implementation - what was ensuring that pods with any NP's applied have free-for-all access? | 17:00 |
| dulek | dmellado: Was that only the groups from pod_security_groups? | 17:00 |
| dulek | ltomasbo: Ah, sorry, missed your message due to disconnection. Yes, you're right. :) | 17:01 |
| dmellado | dulek, if you mean the initial that we implemented yeah. The huawei's patches are actually a different story. I'm actually rewriting the whole devref, but you can still see | 17:02 |
| ltomasbo | dulek, https://github.com/openshift/openshift-ansible/blob/release-3.11/roles/openshift_openstack/templates/heat_stack.yaml.j2#L489-L492 | 17:02 |
| dmellado | it there on the queens blueprint, if you're interested | 17:02 |
| ltomasbo | dulek, though I'm not sure if we are creating the same time of sg anyway... | 17:02 |
| dmellado | I'll submit a patch with the new devref soon | 17:02 |
| dulek | dmellado: Okay, thanks… | 17:03 |
| dmellado | dulek: what would you need that for? | 17:04 |
| dulek | dmellado: We're debugging an issue here, that pods without any SG's applied aren't really having everything open. | 17:04 |
| dulek | ltomasbo, maysams: IMO the correct behavior for DevStack is to create an allow-all SG and add it to pod_security_groups if policy SG driver is enabled. | 17:04 |
| dulek | Do you think this makes sense? | 17:05 |
| dmellado | dulek: you mean you'd like a default-open? | 17:05 |
| dmellado | I'd rather do the opposite, tbh | 17:05 |
| ltomasbo | dulek, but, remove NP and namespace isolation from the equation | 17:05 |
| ltomasbo | dulek, what was enabling the traffic between pods before? that we were creating a SG that allows the traffic between pods with the same sg? | 17:06 |
| dulek | dmellado: Then we wouldn't match how NP's are defined in K8s. | 17:06 |
| dmellado | dulek: should you need that you could just use this | 17:06 |
| dmellado | https://kubernetes.io/docs/concepts/services-networking/network-policies/#default-allow-all-ingress-traffic | 17:06 |
| ltomasbo | dulek, I guess we need to simply change that, so that all ingress is allowed instead | 17:06 |
| dulek | ltomasbo: Without policy all the pods were in the same default SG, so they had connection. | 17:06 |
| ltomasbo | dulek, just insttead of remote_group_id, remote_cidr 0.0.0.0/0 | 17:06 |
| maysams | ltomasbo +1 | 17:07 |
| maysams | that makes sense | 17:07 |
| dmellado | ltomasbo: dulek we could just have devstack apply that policy | 17:07 |
| dmellado | once we create that with policy | 17:07 |
| dulek | ltomasbo: Yes, but I'd rather not touch a DevStack-created default SG and just create my own. | 17:07 |
| dulek | ltomasbo: In the DevStack plugin. | 17:07 |
| ltomasbo | dulek, and now we can simply change that rule so taht it doesn't depend on the other ports having the same SG | 17:07 |
| ltomasbo | dulek, no no, but we can simply create an extra one | 17:07 |
| ltomasbo | default-kuryr-sg | 17:07 |
| ltomasbo | and then tell kuryr-config to use tthat one instead of the devstack default one | 17:08 |
| dulek | ltomasbo: That's my point exactly. | 17:08 |
| dulek | ltomasbo: Yep! | 17:08 |
| dmellado | dulek: yeah, please don't touch the default devstack one | 17:08 |
| dmellado | I'd rather implement that on our plugin as you folks suggest | 17:08 |
| ltomasbo | great! so we all agree xD | 17:08 |
| maysams | :) | 17:09 |
| ltomasbo | anyway, dulek perhaps you can quickly try to change that in your env | 17:09 |
| ltomasbo | just switch that rule, and see if actually is fixing the problem | 17:09 |
| dulek | dmellado: And sorry, I don't understand that allow-all NP. :P | 17:09 |
| * dmellado in meetings where they're speaking about synergies | 17:10 | |
| dulek | dmellado: Unless the expected default behavior for pods without any NP is not "allow-all". :P | 17:10 |
| dulek | ltomasbo: Okay, give me a sec. | 17:10 |
| dmellado | dulek: no worries, we can just use that default-sg and match the default behavior in a cleaner way | 17:10 |
| dulek | maysams: Okay, running that test again with modified default SG. | 17:13 |
| maysams | nice, let's see | 17:13 |
| dmellado | dulek: any issue besides that on the tests? | 17:13 |
| dulek | dmellado: That was the last one failing. :D | 17:14 |
| dmellado | great news | 17:14 |
| dulek | maysams: Sorry. xD | 17:17 |
| dulek | Pod client-a should not be able to connect to service svc-server, but was able to connect. | 17:18 |
| dulek | Pod logs: | 17:18 |
| dulek | svc-server.network-policy-5443 (10.1.0.147:81) open | 17:18 |
| dulek | Now it complains about allowing on port 81. I'll see why this happens. | 17:18 |
| maysams | dulek: remove the default egress rules | 17:18 |
| maysams | dulek: from the client, pls | 17:18 |
| maysams | it might be that | 17:18 |
| dulek | maysams: We add them to any NP SG? | 17:19 |
| dulek | Seems to make sense, let's see if I'll be able to kill the test before namespace is destroyed. :P | 17:19 |
| maysams | no, but when a new sg is created it automatically generates default egress, if I'm not mistaken | 17:20 |
| dulek | maysams: Oh crap. Can we somehow prevent it? | 17:20 |
| ltomasbo | dulek, I'm checking the code in openshift-ansible... and I see the same type of SG being attached to t he pods... not sure how this was working... perhaps it just happen when combining egress and ingress | 17:20 |
| maysams | dulek: I'm not sure, we can always remove it. | 17:20 |
| ltomasbo | and it is happening there too | 17:20 |
| maysams | dulek: as we were not testing egress behavior, we didn't see it before(the default rules). But we also need to add the removal of those rules | 17:23 |
| dulek | maysams: Yep, those are the rules allowing it. :) | 17:24 |
| dulek | maysams: I'll take a look how those are created. | 17:24 |
| maysams | dulek: niceee!! so, all green with the tests? | 17:25 |
| dulek | maysams: I can't really do the run, as deleting the rules would need to be implemented in Kuryr. :P | 17:26 |
| dulek | maysams: Here's how terraform deals with this https://github.com/hashicorp/terraform/pull/11466/files | 17:26 |
| maysams | dulek: I can also take a look on how to remove them when a NP is applied | 17:29 |
| dulek | maysams: Just remove it here: https://github.com/openstack/kuryr-kubernetes/blob/02b51775a4c385badaf9e22965fe19889516406a/kuryr_kubernetes/controller/drivers/network_policy.py#L168 ? :P | 17:30 |
| maysams | dulek, yup! that works | 17:30 |
| dulek | maysams: Fair enough. :) Do you want to work on both those patches or should I take any? | 17:32 |
| ltomasbo | dulek, there is a method to add some default needed rules at line 123 | 17:32 |
| ltomasbo | dulek, perhaps you can add them the egress removal | 17:32 |
| maysams | yup, go for it | 17:32 |
| ltomasbo | dulek, problem is... you still need those rules if there is no egress rules at the NP, right? | 17:33 |
| dulek | ltomasbo: Hm… | 17:34 |
| dulek | ltomasbo: This is okay IMO. Those default rules created by us are only ingress. | 17:36 |
| *** gcheresh has quit IRC | 17:36 | |
| *** gcheresh has joined #openstack-kuryr | 17:45 | |
| *** maysams has quit IRC | 17:57 | |
| openstackgerrit | Michał Dulko proposed openstack/kuryr-kubernetes master: NP: Create allow-all SG and add it to pod SG's https://review.openstack.org/648488 | 18:05 |
| *** celebdor has quit IRC | 18:23 | |
| openstackgerrit | Michał Dulko proposed openstack/kuryr-kubernetes master: NP: Delete default egress rules https://review.openstack.org/648497 | 18:24 |
| *** gcheresh has quit IRC | 18:31 | |
| *** pcaruana has quit IRC | 20:08 | |
| *** openstackgerrit has quit IRC | 21:07 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!