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 <mojtaba-esk@users.noreply.github.com>
celestiaorg · May 17, 2024 · f06c706 · f06c706
1 parent f0d1b11
commit f06c706
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 {