From f06c706301591af4d8b0f93c4c52f30fb98053de Mon Sep 17 00:00:00 2001 From: smuu <18609909+smuu@users.noreply.github.com> Date: Fri, 17 May 2024 11:43:26 +0200 Subject: [PATCH] bugfix: sidecar should use parents service (#346) * bugfix: sidecar should use parents service Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com> * bugfix: assign one var to another Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com> --------- Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com> Co-authored-by: Mojtaba --- pkg/builder/builder.go | 2 +- pkg/k8s/k8s_service.go | 10 +++---- pkg/knuu/errors.go | 2 ++ pkg/knuu/instance.go | 2 +- pkg/knuu/instance_helper.go | 58 ++++++++++++++++++++++++------------- 5 files changed, 47 insertions(+), 27 deletions(-) diff --git a/pkg/builder/builder.go b/pkg/builder/builder.go index 3a4fb04..d30e56b 100644 --- a/pkg/builder/builder.go +++ b/pkg/builder/builder.go @@ -40,7 +40,7 @@ func (c *CacheOptions) Default(buildContext string) (*CacheOptions, error) { Dir: "", // ttl.sh with the hash of build context is used as the cache repo // Kaniko adds a string tag to the image name, so we don't need to add it here - Repo: fmt.Sprintf("ttl.sh/%s", ctxHash), + Repo: fmt.Sprintf("ttl.sh/%s:24h", ctxHash), }, nil } diff --git a/pkg/k8s/k8s_service.go b/pkg/k8s/k8s_service.go index e7ccd99..0074fe6 100644 --- a/pkg/k8s/k8s_service.go +++ b/pkg/k8s/k8s_service.go @@ -46,19 +46,19 @@ func (c *Client) PatchService( selectorMap map[string]string, portsTCP, portsUDP []int, -) error { +) (*v1.Service, error) { svc, err := prepareService(c.namespace, name, labels, selectorMap, portsTCP, portsUDP) if err != nil { - return ErrPreparingService.WithParams(name).Wrap(err) + return nil, ErrPreparingService.WithParams(name).Wrap(err) } - _, err = c.clientset.CoreV1().Services(c.namespace).Update(ctx, svc, metav1.UpdateOptions{}) + serv, err := c.clientset.CoreV1().Services(c.namespace).Update(ctx, svc, metav1.UpdateOptions{}) if err != nil { - return ErrPatchingService.WithParams(name).Wrap(err) + return nil, ErrPatchingService.WithParams(name).Wrap(err) } logrus.Debugf("Service %s patched in namespace %s", name, c.namespace) - return nil + return serv, nil } func (c *Client) DeleteService(ctx context.Context, name string) error { diff --git a/pkg/knuu/errors.go b/pkg/knuu/errors.go index 411c28a..167d663 100644 --- a/pkg/knuu/errors.go +++ b/pkg/knuu/errors.go @@ -62,6 +62,8 @@ var ( ErrFailedToCreateConfigMap = &Error{Code: "FailedToCreateConfigMap", Message: "failed to create configmap"} ErrFailedToDeleteConfigMap = &Error{Code: "FailedToDeleteConfigMap", Message: "failed to delete configmap"} ErrFailedToDeployOrPatchService = &Error{Code: "FailedToDeployOrPatchService", Message: "failed to deploy or patch service"} + ErrDeployingServiceForSidecar = &Error{Code: "DeployingServiceForSidecar", Message: "error deploying service for sidecar '%s' of instance '%s', a sidecar cannot have a service"} + ErrPatchingServiceForSidecar = &Error{Code: "PatchingServiceForSidecar", Message: "error patching service for sidecar '%s' of instance '%s', a sidecar cannot have a service"} ErrDeployingVolumeForInstance = &Error{Code: "DeployingVolumeForInstance", Message: "error deploying volume for instance '%s'"} ErrDeployingFilesForInstance = &Error{Code: "DeployingFilesForInstance", Message: "error deploying files for instance '%s'"} ErrDestroyingVolumeForInstance = &Error{Code: "DestroyingVolumeForInstance", Message: "error destroying volume for instance '%s'"} diff --git a/pkg/knuu/instance.go b/pkg/knuu/instance.go index 10c68da..2a496b8 100644 --- a/pkg/knuu/instance.go +++ b/pkg/knuu/instance.go @@ -743,7 +743,7 @@ func (i *Instance) GetIP() (string, error) { svc, err := k8sClient.GetService(ctx, i.k8sName) if err != nil || svc == nil { // Service does not exist, so we need to deploy it - err := i.deployService(ctx) + err := i.deployService(ctx, i.portsTCP, i.portsUDP) if err != nil { return "", ErrDeployingServiceForInstance.WithParams(i.k8sName).Wrap(err) } diff --git a/pkg/knuu/instance_helper.go b/pkg/knuu/instance_helper.go index 7539d02..daa93be 100644 --- a/pkg/knuu/instance_helper.go +++ b/pkg/knuu/instance_helper.go @@ -80,11 +80,17 @@ func (i *Instance) Labels() map[string]string { } // deployService deploys the service for the instance -func (i *Instance) deployService(ctx context.Context) error { +func (i *Instance) deployService(ctx context.Context, portsTCP, portsUDP []int) error { + // a sidecar instance should use the parent instance's service + if i.isSidecar { + return ErrDeployingServiceForSidecar.WithParams(i.k8sName) + } + + serviceName := i.k8sName labels := i.getLabels() - selectorMap := i.getLabels() + labelSelectors := labels - service, err := k8sClient.CreateService(ctx, i.k8sName, labels, selectorMap, i.portsTCP, i.portsUDP) + service, err := k8sClient.CreateService(ctx, serviceName, labels, labelSelectors, portsTCP, portsUDP) if err != nil { return ErrDeployingService.WithParams(i.k8sName).Wrap(err) } @@ -94,19 +100,22 @@ func (i *Instance) deployService(ctx context.Context) error { } // patchService patches the service for the instance -func (i *Instance) patchService(ctx context.Context) error { - if i.kubernetesService == nil { - svc, err := k8sClient.GetService(ctx, i.k8sName) - if err != nil { - return ErrGettingService.WithParams(i.k8sName).Wrap(err) - } - i.kubernetesService = svc +func (i *Instance) patchService(ctx context.Context, portsTCP, portsUDP []int) error { + // a sidecar instance should use the parent instance's service + if i.isSidecar { + return ErrPatchingServiceForSidecar.WithParams(i.k8sName) } - err := k8sClient.PatchService(ctx, i.k8sName, i.kubernetesService.ObjectMeta.Labels, i.kubernetesService.Spec.Selector, i.portsTCP, i.portsUDP) + + serviceName := i.k8sName + labels := i.getLabels() + labelSelectors := labels + + service, err := k8sClient.PatchService(ctx, serviceName, labels, labelSelectors, portsTCP, portsUDP) if err != nil { - return ErrPatchingService.WithParams(i.k8sName).Wrap(err) + return ErrPatchingService.WithParams(serviceName).Wrap(err) } - logrus.Debugf("Patched service '%s'", i.k8sName) + i.kubernetesService = service + logrus.Debugf("Patched service '%s'", serviceName) return nil } @@ -180,17 +189,17 @@ func (i *Instance) destroyPod(ctx context.Context) error { } // deployService deploys the service for the instance -func (i *Instance) deployOrPatchService(ctx context.Context) error { - if len(i.portsTCP) != 0 || len(i.portsUDP) != 0 { +func (i *Instance) deployOrPatchService(ctx context.Context, portsTCP, portsUDP []int) error { + if len(portsTCP) != 0 || len(portsUDP) != 0 { logrus.Debugf("Ports not empty, deploying service for instance '%s'", i.k8sName) svc, _ := k8sClient.GetService(ctx, i.k8sName) if svc == nil { - err := i.deployService(ctx) + err := i.deployService(ctx, portsTCP, portsUDP) if err != nil { return ErrDeployingServiceForInstance.WithParams(i.k8sName).Wrap(err) } } else if svc != nil { - err := i.patchService(ctx) + err := i.patchService(ctx, portsTCP, portsUDP) if err != nil { return ErrPatchingServiceForInstance.WithParams(i.k8sName).Wrap(err) } @@ -263,9 +272,18 @@ func (i *Instance) destroyFiles(ctx context.Context) error { // deployResources deploys the resources for the instance func (i *Instance) deployResources(ctx context.Context) error { - if len(i.portsTCP) != 0 || len(i.portsUDP) != 0 { - if err := i.deployOrPatchService(ctx); err != nil { - return ErrFailedToDeployOrPatchService.Wrap(err) + // only a non-sidecar instance should deploy a service, all sidecars will use the parent instance's service + if !i.isSidecar { + portsTCP := i.portsTCP + portsUDP := i.portsUDP + for _, sidecar := range i.sidecars { + portsTCP = append(portsTCP, sidecar.portsTCP...) + portsUDP = append(portsUDP, sidecar.portsUDP...) + } + if len(portsTCP) != 0 || len(portsUDP) != 0 { + if err := i.deployOrPatchService(ctx, portsTCP, portsUDP); err != nil { + return ErrFailedToDeployOrPatchService.Wrap(err) + } } } if len(i.volumes) != 0 {