From d48c00418ae7f6efcfe45d92f3c5e08b9c8afacd Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 19 Nov 2021 09:36:40 +0100 Subject: Move a comment to the relevant place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- libpod/kube.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libpod/kube.go b/libpod/kube.go index 351c49c9a..30a3b069c 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -270,6 +270,7 @@ func GenerateKubeServiceFromV1Pod(pod *v1.Pod, servicePorts []v1.ServicePort) YA func containerPortsToServicePorts(containerPorts []v1.ContainerPort) []v1.ServicePort { sps := make([]v1.ServicePort, 0, len(containerPorts)) for _, cp := range containerPorts { + // Legal nodeport range is 30000-32767 nodePort := 30000 + rand.Intn(32767-30000+1) servicePort := v1.ServicePort{ Protocol: cp.Protocol, @@ -287,7 +288,7 @@ func containerPortsToServicePorts(containerPorts []v1.ContainerPort) []v1.Servic // inclusive list of serviceports to expose func containersToServicePorts(containers []v1.Container) []v1.ServicePort { // Without the call to rand.Seed, a program will produce the same sequence of pseudo-random numbers - // for each execution. Legal nodeport range is 30000-32767 + // for each execution. rand.Seed(time.Now().UnixNano()) sps := make([]v1.ServicePort, 0, len(containers)) -- cgit v1.2.3-54-g00ecf From ec686d68ddf326c8f4e50f2770be3ed7d250c6d0 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 19 Nov 2021 09:52:23 +0100 Subject: Don't use the global math/rand RNG for service ports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use a private RNG with the desired seed, don't interfere with the other uses. Introducing the servicePortState type is rather overkill for the single member, but we'll add another one immediately. Signed-off-by: Miloslav Trmač --- libpod/kube.go | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/libpod/kube.go b/libpod/kube.go index 30a3b069c..73d219fbc 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -79,7 +79,8 @@ func (p *Pod) GenerateForKube(ctx context.Context) (*v1.Pod, []v1.ServicePort, e if err != nil { return nil, servicePorts, err } - servicePorts = containerPortsToServicePorts(ports) + spState := newServicePortState() + servicePorts = spState.containerPortsToServicePorts(ports) hostNetwork = infraContainer.NetworkMode() == string(namespaces.NetworkMode(specgen.Host)) } pod, err := p.podWithContainers(ctx, allContainers, ports, hostNetwork) @@ -265,13 +266,26 @@ func GenerateKubeServiceFromV1Pod(pod *v1.Pod, servicePorts []v1.ServicePort) YA return service } +// servicePortState allows calling containerPortsToServicePorts for a single service +type servicePortState struct { + // A program using the shared math/rand state with the default seed will produce the same sequence of pseudo-random numbers + // for each execution. Use a private RNG state not to interfere with other users. + rng *rand.Rand +} + +func newServicePortState() servicePortState { + return servicePortState{ + rng: rand.New(rand.NewSource(time.Now().UnixNano())), + } +} + // containerPortsToServicePorts takes a slice of containerports and generates a // slice of service ports -func containerPortsToServicePorts(containerPorts []v1.ContainerPort) []v1.ServicePort { +func (state *servicePortState) containerPortsToServicePorts(containerPorts []v1.ContainerPort) []v1.ServicePort { sps := make([]v1.ServicePort, 0, len(containerPorts)) for _, cp := range containerPorts { // Legal nodeport range is 30000-32767 - nodePort := 30000 + rand.Intn(32767-30000+1) + nodePort := 30000 + state.rng.Intn(32767-30000+1) servicePort := v1.ServicePort{ Protocol: cp.Protocol, Port: cp.ContainerPort, @@ -287,13 +301,10 @@ func containerPortsToServicePorts(containerPorts []v1.ContainerPort) []v1.Servic // containersToServicePorts takes a slice of v1.Containers and generates an // inclusive list of serviceports to expose func containersToServicePorts(containers []v1.Container) []v1.ServicePort { - // Without the call to rand.Seed, a program will produce the same sequence of pseudo-random numbers - // for each execution. - rand.Seed(time.Now().UnixNano()) - + state := newServicePortState() sps := make([]v1.ServicePort, 0, len(containers)) for _, ctr := range containers { - sps = append(sps, containerPortsToServicePorts(ctr.Ports)...) + sps = append(sps, state.containerPortsToServicePorts(ctr.Ports)...) } return sps } -- cgit v1.2.3-54-g00ecf From a948ecbb9c7c90e249cded94a2239192a4ad766a Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 19 Nov 2021 09:58:26 +0100 Subject: Allow containerPortsToServicePorts to fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an error return to it and affected callers. Should not affect behavior, the function can't currently fail. Signed-off-by: Miloslav Trmač --- libpod/kube.go | 29 ++++++++++++++++++++--------- pkg/domain/infra/abi/generate.go | 12 ++++++++++-- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/libpod/kube.go b/libpod/kube.go index 73d219fbc..bdd5b9a4f 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -80,7 +80,10 @@ func (p *Pod) GenerateForKube(ctx context.Context) (*v1.Pod, []v1.ServicePort, e return nil, servicePorts, err } spState := newServicePortState() - servicePorts = spState.containerPortsToServicePorts(ports) + servicePorts, err = spState.containerPortsToServicePorts(ports) + if err != nil { + return nil, servicePorts, err + } hostNetwork = infraContainer.NetworkMode() == string(namespaces.NetworkMode(specgen.Host)) } pod, err := p.podWithContainers(ctx, allContainers, ports, hostNetwork) @@ -243,13 +246,17 @@ func ConvertV1PodToYAMLPod(pod *v1.Pod) *YAMLPod { } // GenerateKubeServiceFromV1Pod creates a v1 service object from a v1 pod object -func GenerateKubeServiceFromV1Pod(pod *v1.Pod, servicePorts []v1.ServicePort) YAMLService { +func GenerateKubeServiceFromV1Pod(pod *v1.Pod, servicePorts []v1.ServicePort) (YAMLService, error) { service := YAMLService{} selector := make(map[string]string) selector["app"] = pod.Labels["app"] ports := servicePorts if len(ports) == 0 { - ports = containersToServicePorts(pod.Spec.Containers) + p, err := containersToServicePorts(pod.Spec.Containers) + if err != nil { + return service, err + } + ports = p } serviceSpec := v1.ServiceSpec{ Ports: ports, @@ -263,7 +270,7 @@ func GenerateKubeServiceFromV1Pod(pod *v1.Pod, servicePorts []v1.ServicePort) YA APIVersion: pod.TypeMeta.APIVersion, } service.TypeMeta = tm - return service + return service, nil } // servicePortState allows calling containerPortsToServicePorts for a single service @@ -281,7 +288,7 @@ func newServicePortState() servicePortState { // containerPortsToServicePorts takes a slice of containerports and generates a // slice of service ports -func (state *servicePortState) containerPortsToServicePorts(containerPorts []v1.ContainerPort) []v1.ServicePort { +func (state *servicePortState) containerPortsToServicePorts(containerPorts []v1.ContainerPort) ([]v1.ServicePort, error) { sps := make([]v1.ServicePort, 0, len(containerPorts)) for _, cp := range containerPorts { // Legal nodeport range is 30000-32767 @@ -295,18 +302,22 @@ func (state *servicePortState) containerPortsToServicePorts(containerPorts []v1. } sps = append(sps, servicePort) } - return sps + return sps, nil } // containersToServicePorts takes a slice of v1.Containers and generates an // inclusive list of serviceports to expose -func containersToServicePorts(containers []v1.Container) []v1.ServicePort { +func containersToServicePorts(containers []v1.Container) ([]v1.ServicePort, error) { state := newServicePortState() sps := make([]v1.ServicePort, 0, len(containers)) for _, ctr := range containers { - sps = append(sps, state.containerPortsToServicePorts(ctr.Ports)...) + ports, err := state.containerPortsToServicePorts(ctr.Ports) + if err != nil { + return nil, err + } + sps = append(sps, ports...) } - return sps + return sps, nil } func (p *Pod) podWithContainers(ctx context.Context, containers []*Container, ports []v1.ContainerPort, hostNetwork bool) (*v1.Pod, error) { diff --git a/pkg/domain/infra/abi/generate.go b/pkg/domain/infra/abi/generate.go index 0defa1923..68bb351bf 100644 --- a/pkg/domain/infra/abi/generate.go +++ b/pkg/domain/infra/abi/generate.go @@ -139,7 +139,11 @@ func (ic *ContainerEngine) GenerateKube(ctx context.Context, nameOrIDs []string, podContent = append(podContent, b) if options.Service { - b, err := generateKubeYAML(libpod.GenerateKubeServiceFromV1Pod(po, []k8sAPI.ServicePort{})) + svc, err := libpod.GenerateKubeServiceFromV1Pod(po, []k8sAPI.ServicePort{}) + if err != nil { + return nil, err + } + b, err := generateKubeYAML(svc) if err != nil { return nil, err } @@ -177,7 +181,11 @@ func getKubePods(ctx context.Context, pods []*libpod.Pod, getService bool) ([][] pos = append(pos, b) if getService { - b, err := generateKubeYAML(libpod.GenerateKubeServiceFromV1Pod(po, sp)) + svc, err := libpod.GenerateKubeServiceFromV1Pod(po, sp) + if err != nil { + return nil, nil, err + } + b, err := generateKubeYAML(svc) if err != nil { return nil, nil, err } -- cgit v1.2.3-54-g00ecf From f415b3055290b383c76d98127ec168ddea629680 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 19 Nov 2021 10:01:02 +0100 Subject: Ensure the generated NodePort values are unique MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... at least within a single service. [NO NEW TESTS NEEDED] because testing RNGs is problematic. (We _could_ probably inject a mock RNG implementation that always returns the same value, or something like that.) Signed-off-by: Miloslav Trmač --- libpod/kube.go | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/libpod/kube.go b/libpod/kube.go index bdd5b9a4f..4e61b5377 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -277,12 +277,14 @@ func GenerateKubeServiceFromV1Pod(pod *v1.Pod, servicePorts []v1.ServicePort) (Y type servicePortState struct { // A program using the shared math/rand state with the default seed will produce the same sequence of pseudo-random numbers // for each execution. Use a private RNG state not to interfere with other users. - rng *rand.Rand + rng *rand.Rand + usedPorts map[int]struct{} } func newServicePortState() servicePortState { return servicePortState{ - rng: rand.New(rand.NewSource(time.Now().UnixNano())), + rng: rand.New(rand.NewSource(time.Now().UnixNano())), + usedPorts: map[int]struct{}{}, } } @@ -291,8 +293,20 @@ func newServicePortState() servicePortState { func (state *servicePortState) containerPortsToServicePorts(containerPorts []v1.ContainerPort) ([]v1.ServicePort, error) { sps := make([]v1.ServicePort, 0, len(containerPorts)) for _, cp := range containerPorts { - // Legal nodeport range is 30000-32767 - nodePort := 30000 + state.rng.Intn(32767-30000+1) + var nodePort int + attempt := 0 + for { + // Legal nodeport range is 30000-32767 + nodePort = 30000 + state.rng.Intn(32767-30000+1) + if _, found := state.usedPorts[nodePort]; !found { + state.usedPorts[nodePort] = struct{}{} + break + } + attempt++ + if attempt >= 100 { + return nil, fmt.Errorf("too many attempts trying to generate a unique NodePort number") + } + } servicePort := v1.ServicePort{ Protocol: cp.Protocol, Port: cp.ContainerPort, -- cgit v1.2.3-54-g00ecf