control-plane: automatically create Dataplane resource on k8s and use to generate inbound Envoy listeners #104
Conversation
yskopets
force-pushed
the
feature/generate-dataplane-out-of-pod
branch
6 times, most recently
from
9d64503
to
6176f99
Compare
yskopets
changed the title
| }, | ||
| Entry("dataplane IP address is missing", testCase{ | ||
| input: ":80:8080", | ||
| expectedErr: `invalid format: expected ^(?P<workload_ip>(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)):(?P<service_port>[0-9]{1,5}):(?P<workload_port>[0-9]{1,5})$, got ":80:8080"`, |
There was a problem hiding this comment.
in whole file: can we extract this regex to const or reuse one from production code?
There was a problem hiding this comment.
fixed (by relaxing error matcher)
components/konvoy-control-plane/pkg/plugins/discovery/k8s/controllers/dataplane_controller.go
Show resolved
Hide resolved
| Spec: kube_core.ServiceSpec{ | ||
| Ports: []kube_core.ServicePort{ | ||
| { | ||
| Port: 80, |
There was a problem hiding this comment.
is the protocol implicit here?
| Spec: kube_core.ServiceSpec{ | ||
| Ports: []kube_core.ServicePort{ | ||
| { | ||
| Protocol: "UDP", |
There was a problem hiding this comment.
It's nice to give comments why those are mismatching so:
Protocol: "UDP", // only TCP is supported
Protocol: "SCTP", // only TCP is supported
IntVal: 7071, // no container with such port
StrVal: "diagnostics", // no port with that name
| ) | ||
| }) | ||
|
|
||
| var _ = Describe("DataplaneFor(..)", func() { |
There was a problem hiding this comment.
Those cases for this are already fully covered in Describe("PodToDataplane(..)") do we need second tests for this?
When we will have to change of the DataplaneFor we would have to change tests for both cases.
There was a problem hiding this comment.
removed separate test for DataplaneFor(..)
|
|
||
| func MatchServiceThatSelectsPod(pod *kube_core.Pod) ServicePredicate { | ||
| return func(svc *kube_core.Service) bool { | ||
| selector := kube_labels.SelectorFromSet(kube_labels.Set(svc.Spec.Selector)) |
There was a problem hiding this comment.
It seems that kube_labels.Set() is redundant conversion.
There was a problem hiding this comment.
removed explicit type conversion
components/konvoy-control-plane/pkg/plugins/discovery/k8s/controllers/pod_controller.go
Show resolved
Hide resolved
| Name: pod.Name, | ||
| }, | ||
| } | ||
| op, err := kube_controllerutil.CreateOrUpdate(ctx, r.Client, dataplane, func() error { |
There was a problem hiding this comment.
I had to go to editor to see what op is. Can we give it a bit more descriptive name like operationResult or operationRes. I would be also confused to see op in logs.
components/konvoy-control-plane/pkg/plugins/discovery/k8s/controllers/pod_converter.go
Show resolved
Hide resolved
| ) | ||
|
|
||
| var _ DiscoverySource = &DiscoverySink{} | ||
| var _ DiscoveryConsumer = &DiscoverySink{} | ||
|
|
||
| // DiscoverySink is both a source and a consumer of discovery information. | ||
| type DiscoverySink struct { | ||
| Consumer DiscoveryConsumer | ||
| ServiceConsumer ServiceDiscoveryConsumer | ||
| WorkloadConsumer WorkloadDiscoveryConsumer |
There was a problem hiding this comment.
What do you think about removing ServiceConsumer and WorkloadConsumer?
There is no usage of this - no source of those events and no consumer.
…d with Konvoy sidecar injected
yskopets
force-pushed
the
feature/generate-dataplane-out-of-pod
branch
from
8f38d45
to
6b71548
Compare
components/konvoy-control-plane/pkg/plugins/discovery/k8s/controllers/dataplane_controller.go
Show resolved
Hide resolved
changes: