From 5ab858dc6ad7b9116de5547e1d44de4f9314111a Mon Sep 17 00:00:00 2001 From: Moji Date: Thu, 19 Sep 2024 11:45:32 +0330 Subject: [PATCH] chore!: use unique static names for instances (#530) * chore!: refactor to use unique static names for instances * fix: the name collision issue for sidecars * fix!: name collision issue for sidecar cloning * Update pkg/instance/instance.go Co-authored-by: tty47 <32740567+tty47@users.noreply.github.com> --------- Co-authored-by: tty47 <32740567+tty47@users.noreply.github.com> --- e2e/tshark/tshark_test.go | 2 +- pkg/builder/kaniko/kaniko.go | 2 +- pkg/builder/kaniko/kaniko_test.go | 2 +- pkg/instance/build.go | 2 +- pkg/instance/errors.go | 2 + pkg/instance/execution.go | 78 +++++++++++------------ pkg/instance/instance.go | 80 +++++++++--------------- pkg/instance/network.go | 51 +++++++-------- pkg/instance/proxy.go | 4 +- pkg/instance/resources.go | 14 ++--- pkg/instance/sidecars.go | 20 +++--- pkg/instance/storage.go | 14 ++--- pkg/knuu/instance.go | 4 +- pkg/knuu/knuu.go | 4 +- pkg/knuu/knuu_test.go | 3 - pkg/preloader/errors.go | 2 +- pkg/preloader/preloader.go | 32 +++++----- pkg/sidecars/netshaper/netshaper.go | 14 +++-- pkg/sidecars/netshaper/netshaper_test.go | 26 ++++---- pkg/sidecars/observability/obsy.go | 14 +++-- pkg/sidecars/observability/obsy_test.go | 15 +++-- pkg/sidecars/tshark/tshark.go | 16 +++-- pkg/sidecars/tshark/tshark_test.go | 13 ++-- pkg/system/dependencies.go | 16 +++++ 24 files changed, 229 insertions(+), 201 deletions(-) diff --git a/e2e/tshark/tshark_test.go b/e2e/tshark/tshark_test.go index ccce5d00..2acdebdf 100644 --- a/e2e/tshark/tshark_test.go +++ b/e2e/tshark/tshark_test.go @@ -78,7 +78,7 @@ func TestTshark(t *testing.T) { require.NoError(t, target.Sidecars().Add(ctx, tsc)) var ( - filename = tsc.Instance().K8sName() + tshark.TsharkCaptureFileExtension + filename = tsc.Instance().Name() + tshark.TsharkCaptureFileExtension fileKey = filepath.Join(keyPrefix, filename) ) diff --git a/pkg/builder/kaniko/kaniko.go b/pkg/builder/kaniko/kaniko.go index 61a119bd..026590b4 100644 --- a/pkg/builder/kaniko/kaniko.go +++ b/pkg/builder/kaniko/kaniko.go @@ -30,7 +30,7 @@ const ( ) type Kaniko struct { - system.SystemDependencies + *system.SystemDependencies } var _ builder.Builder = &Kaniko{} diff --git a/pkg/builder/kaniko/kaniko_test.go b/pkg/builder/kaniko/kaniko_test.go index f440c10b..8b75b378 100644 --- a/pkg/builder/kaniko/kaniko_test.go +++ b/pkg/builder/kaniko/kaniko_test.go @@ -30,7 +30,7 @@ func TestKanikoBuilder(t *testing.T) { k8sClient, err := k8s.NewClientCustom(context.Background(), k8sCS, k8sCS.Discovery(), nil, k8sNamespace, logrus.New()) require.NoError(t, err) kb := &Kaniko{ - SystemDependencies: system.SystemDependencies{ + SystemDependencies: &system.SystemDependencies{ K8sClient: k8sClient, }, } diff --git a/pkg/instance/build.go b/pkg/instance/build.go index 0d2f0850..56990d20 100644 --- a/pkg/instance/build.go +++ b/pkg/instance/build.go @@ -215,7 +215,7 @@ func getImageRegistry(imageName string) (string, error) { // getBuildDir returns the build directory for the instance func (b *build) getBuildDir() string { - return filepath.Join(buildDirBase, b.instance.k8sName) + return filepath.Join(buildDirBase, b.instance.name) } // addFileToBuilder adds a file to the builder diff --git a/pkg/instance/errors.go b/pkg/instance/errors.go index 8f911de3..87982a0a 100644 --- a/pkg/instance/errors.go +++ b/pkg/instance/errors.go @@ -217,4 +217,6 @@ var ( ErrSidecarInstanceIsNil = errors.New("SidecarInstanceIsNil", "sidecar instance is nil for instance '%s'") ErrFailedToDeletePersistentVolumeClaim = errors.New("FailedToDeletePersistentVolumeClaim", "failed to delete persistent volume claim") ErrUpgradingImageNotAllowed = errors.New("UpgradingImageNotAllowed", "upgrading image is only allowed in state 'Started'. Current state is '%s'") + ErrInstanceNameAlreadyExists = errors.New("InstanceNameAlreadyExists", "instance name '%s' already exists") + ErrSettingSidecarName = errors.New("SettingSidecarName", "error setting sidecar name with prefix '%s' for instance '%s'") ) diff --git a/pkg/instance/execution.go b/pkg/instance/execution.go index 620ccf71..7b5cc0ba 100644 --- a/pkg/instance/execution.go +++ b/pkg/instance/execution.go @@ -40,20 +40,20 @@ func (e *execution) ExecuteCommand(ctx context.Context, command ...string) (stri var ( instanceName string eErr *Error - containerName = e.instance.k8sName + containerName = e.instance.name ) if e.instance.sidecars.isSidecar { - instanceName = e.instance.parentInstance.k8sName - eErr = ErrExecutingCommandInSidecar.WithParams(command, e.instance.k8sName, e.instance.parentInstance.k8sName) + instanceName = e.instance.parentInstance.name + eErr = ErrExecutingCommandInSidecar.WithParams(command, e.instance.name, e.instance.parentInstance.name) } else { - instanceName = e.instance.k8sName - eErr = ErrExecutingCommandInInstance.WithParams(command, e.instance.k8sName) + instanceName = e.instance.name + eErr = ErrExecutingCommandInInstance.WithParams(command, e.instance.name) } pod, err := e.instance.K8sClient.GetFirstPodFromReplicaSet(ctx, instanceName) if err != nil { - return "", ErrGettingPodFromReplicaSet.WithParams(e.instance.k8sName).Wrap(err) + return "", ErrGettingPodFromReplicaSet.WithParams(e.instance.name).Wrap(err) } commandWithShell := []string{"/bin/sh", "-c", strings.Join(command, " ")} @@ -73,7 +73,7 @@ func (e *execution) StartWithCallback(ctx context.Context, callback func()) erro go func() { err := e.WaitInstanceIsRunning(ctx) if err != nil { - e.instance.Logger.WithError(err).WithField("instance", e.instance.k8sName).Error("waiting for instance to be running") + e.instance.Logger.WithError(err).WithField("instance", e.instance.name).Error("waiting for instance to be running") return } callback() @@ -85,7 +85,7 @@ func (e *execution) StartWithCallback(ctx context.Context, callback func()) erro // This function can only be called in the state 'Committed' or 'Stopped' func (e *execution) StartAsync(ctx context.Context) error { if !e.instance.IsInState(StateCommitted, StateStopped) { - return ErrStartingNotAllowed.WithParams(e.instance.k8sName, e.instance.state.String()) + return ErrStartingNotAllowed.WithParams(e.instance.name, e.instance.state.String()) } if err := e.instance.sidecars.verifySidecarsStates(); err != nil { @@ -109,12 +109,12 @@ func (e *execution) StartAsync(ctx context.Context) error { if e.instance.state == StateCommitted { if err := e.deployResourcesForCommittedState(ctx); err != nil { - return ErrDeployingResourcesForInstance.WithParams(e.instance.k8sName).Wrap(err) + return ErrDeployingResourcesForInstance.WithParams(e.instance.name).Wrap(err) } } if err := e.deployPod(ctx); err != nil { - return ErrDeployingPodForInstance.WithParams(e.instance.k8sName).Wrap(err) + return ErrDeployingPodForInstance.WithParams(e.instance.name).Wrap(err) } e.instance.SetState(StateStarted) @@ -130,7 +130,7 @@ func (e *execution) Start(ctx context.Context) error { } if err := e.WaitInstanceIsRunning(ctx); err != nil { - return ErrWaitingForInstanceRunning.WithParams(e.instance.k8sName).Wrap(err) + return ErrWaitingForInstanceRunning.WithParams(e.instance.name).Wrap(err) } return nil } @@ -142,7 +142,7 @@ func (e *execution) IsRunning(ctx context.Context) (bool, error) { return false, ErrCheckingIfInstanceRunningNotAllowed.WithParams(e.instance.state.String()) } - return e.instance.K8sClient.IsReplicaSetRunning(ctx, e.instance.k8sName) + return e.instance.K8sClient.IsReplicaSetRunning(ctx, e.instance.name) } // WaitInstanceIsRunning waits until the instance is running @@ -155,7 +155,7 @@ func (e *execution) WaitInstanceIsRunning(ctx context.Context) error { for { running, err := e.IsRunning(ctx) if err != nil { - return ErrCheckingIfInstanceRunning.WithParams(e.instance.k8sName).Wrap(err) + return ErrCheckingIfInstanceRunning.WithParams(e.instance.name).Wrap(err) } if running { return nil @@ -164,7 +164,7 @@ func (e *execution) WaitInstanceIsRunning(ctx context.Context) error { select { case <-ctx.Done(): return ErrWaitingForInstanceTimeout. - WithParams(e.instance.k8sName).Wrap(ctx.Err()) + WithParams(e.instance.name).Wrap(ctx.Err()) case <-time.After(waitForInstanceRetry): continue } @@ -183,13 +183,13 @@ func (e *execution) WaitInstanceIsStopped(ctx context.Context) error { break } if err != nil { - return ErrCheckingIfInstanceStopped.WithParams(e.instance.k8sName).Wrap(err) + return ErrCheckingIfInstanceStopped.WithParams(e.instance.name).Wrap(err) } select { case <-ctx.Done(): return ErrWaitingForInstanceTimeout. - WithParams(e.instance.k8sName).Wrap(ctx.Err()) + WithParams(e.instance.name).Wrap(ctx.Err()) case <-time.After(waitForInstanceRetry): continue } @@ -208,7 +208,7 @@ func (e *execution) Stop(ctx context.Context) error { } if err := e.destroyPod(ctx); err != nil { - return ErrDestroyingPod.WithParams(e.instance.k8sName).Wrap(err) + return ErrDestroyingPod.WithParams(e.instance.name).Wrap(err) } e.instance.SetState(StateStopped) @@ -219,12 +219,12 @@ func (e *execution) Stop(ctx context.Context) error { // Labels returns the labels for the instance func (e *execution) Labels() map[string]string { return map[string]string{ - labelAppKey: e.instance.k8sName, + labelAppKey: e.instance.name, labelManagedByKey: labelKnuuValue, labelScopeKey: e.instance.Scope, labelTestStartedKey: e.instance.StartTime, labelNameKey: e.instance.name, - labelK8sNameKey: e.instance.k8sName, + labelK8sNameKey: e.instance.name, labelTypeKey: e.instance.instanceType.String(), } } @@ -241,22 +241,22 @@ func (e *execution) Destroy(ctx context.Context) error { } if err := e.destroyPod(ctx); err != nil { - return ErrDestroyingPod.WithParams(e.instance.k8sName).Wrap(err) + return ErrDestroyingPod.WithParams(e.instance.name).Wrap(err) } if err := e.instance.resources.destroyResources(ctx); err != nil { - return ErrDestroyingResourcesForInstance.WithParams(e.instance.k8sName).Wrap(err) + return ErrDestroyingResourcesForInstance.WithParams(e.instance.name).Wrap(err) } err := e.instance.sidecars.applyFunctionToSidecars( func(sidecar SidecarManager) error { e.instance.Logger.WithFields(logrus.Fields{ - "instance": e.instance.k8sName, - "sidecar": sidecar.Instance().k8sName, + "instance": e.instance.name, + "sidecar": sidecar.Instance().name, }).Debugf("destroying sidecar resources") return sidecar.Instance().resources.destroyResources(ctx) }) if err != nil { - return ErrDestroyingResourcesForSidecars.WithParams(e.instance.k8sName).Wrap(err) + return ErrDestroyingResourcesForSidecars.WithParams(e.instance.name).Wrap(err) } e.instance.SetState(StateDestroyed) @@ -297,7 +297,7 @@ func BatchDestroy(ctx context.Context, instances ...*Instance) error { // deployResourcesForCommittedState handles resource deployment for instances in the 'Committed' state func (e *execution) deployResourcesForCommittedState(ctx context.Context) error { if err := e.instance.resources.deployResources(ctx); err != nil { - return ErrDeployingResourcesForInstance.WithParams(e.instance.k8sName).Wrap(err) + return ErrDeployingResourcesForInstance.WithParams(e.instance.name).Wrap(err) } err := e.instance.sidecars.applyFunctionToSidecars(func(sc SidecarManager) error { if err := sc.PreStart(ctx); err != nil { @@ -306,7 +306,7 @@ func (e *execution) deployResourcesForCommittedState(ctx context.Context) error return sc.Instance().resources.deployResources(ctx) }) if err != nil { - return ErrDeployingResourcesForSidecars.WithParams(e.instance.k8sName).Wrap(err) + return ErrDeployingResourcesForSidecars.WithParams(e.instance.name).Wrap(err) } return nil @@ -318,16 +318,16 @@ func (e *execution) deployPod(ctx context.Context) error { labels := e.Labels() // create a service account for the pod - if err := e.instance.K8sClient.CreateServiceAccount(ctx, e.instance.k8sName, labels); err != nil { + if err := e.instance.K8sClient.CreateServiceAccount(ctx, e.instance.name, labels); err != nil { return ErrFailedToCreateServiceAccount.Wrap(err) } // create a role and role binding for the pod if there are policy rules if len(e.instance.security.policyRules) > 0 { - if err := e.instance.K8sClient.CreateRole(ctx, e.instance.k8sName, labels, e.instance.security.policyRules); err != nil { + if err := e.instance.K8sClient.CreateRole(ctx, e.instance.name, labels, e.instance.security.policyRules); err != nil { return ErrFailedToCreateRole.Wrap(err) } - if err := e.instance.K8sClient.CreateRoleBinding(ctx, e.instance.k8sName, labels, e.instance.k8sName, e.instance.k8sName); err != nil { + if err := e.instance.K8sClient.CreateRoleBinding(ctx, e.instance.name, labels, e.instance.name, e.instance.name); err != nil { return ErrFailedToCreateRoleBinding.Wrap(err) } } @@ -342,20 +342,20 @@ func (e *execution) deployPod(ctx context.Context) error { e.instance.kubernetesReplicaSet = replicaSet // Log the deployment of the pod - e.instance.Logger.WithField("instance", e.instance.k8sName).Debugf("started statefulSet") + e.instance.Logger.WithField("instance", e.instance.name).Debugf("started statefulSet") return nil } // destroyPod destroys the pod for the instance (no grace period) // Skips if the pod is already destroyed func (e *execution) destroyPod(ctx context.Context) error { - err := e.instance.K8sClient.DeleteReplicaSetWithGracePeriod(ctx, e.instance.k8sName, nil) + err := e.instance.K8sClient.DeleteReplicaSetWithGracePeriod(ctx, e.instance.name, nil) if err != nil { return ErrFailedToDeletePod.Wrap(err) } // Delete the service account for the pod - if err := e.instance.K8sClient.DeleteServiceAccount(ctx, e.instance.k8sName); err != nil { + if err := e.instance.K8sClient.DeleteServiceAccount(ctx, e.instance.name); err != nil { return ErrFailedToDeleteServiceAccount.Wrap(err) } @@ -364,10 +364,10 @@ func (e *execution) destroyPod(ctx context.Context) error { return nil } - if err := e.instance.K8sClient.DeleteRole(ctx, e.instance.k8sName); err != nil { + if err := e.instance.K8sClient.DeleteRole(ctx, e.instance.name); err != nil { return ErrFailedToDeleteRole.Wrap(err) } - if err := e.instance.K8sClient.DeleteRoleBinding(ctx, e.instance.k8sName); err != nil { + if err := e.instance.K8sClient.DeleteRoleBinding(ctx, e.instance.name); err != nil { return ErrFailedToDeleteRoleBinding.Wrap(err) } @@ -377,7 +377,7 @@ func (e *execution) destroyPod(ctx context.Context) error { // prepareConfig prepares the config for the instance func (e *execution) prepareReplicaSetConfig() k8s.ReplicaSetConfig { containerConfig := k8s.ContainerConfig{ - Name: e.instance.k8sName, + Name: e.instance.name, Image: e.instance.build.imageName, ImagePullPolicy: e.instance.build.imagePullPolicy, Command: e.instance.build.command, @@ -397,7 +397,7 @@ func (e *execution) prepareReplicaSetConfig() k8s.ReplicaSetConfig { sidecarConfigs := make([]k8s.ContainerConfig, 0) for _, sidecar := range e.instance.sidecars.sidecars { sidecarConfigs = append(sidecarConfigs, k8s.ContainerConfig{ - Name: sidecar.Instance().k8sName, + Name: sidecar.Instance().name, Image: sidecar.Instance().build.imageName, Command: sidecar.Instance().build.command, Args: sidecar.Instance().build.args, @@ -416,9 +416,9 @@ func (e *execution) prepareReplicaSetConfig() k8s.ReplicaSetConfig { podConfig := k8s.PodConfig{ Namespace: e.instance.K8sClient.Namespace(), - Name: e.instance.k8sName, + Name: e.instance.name, Labels: e.Labels(), - ServiceAccountName: e.instance.k8sName, + ServiceAccountName: e.instance.name, FsGroup: e.instance.storage.fsGroup, ContainerConfig: containerConfig, SidecarConfigs: sidecarConfigs, @@ -426,7 +426,7 @@ func (e *execution) prepareReplicaSetConfig() k8s.ReplicaSetConfig { return k8s.ReplicaSetConfig{ Namespace: e.instance.K8sClient.Namespace(), - Name: e.instance.k8sName, + Name: e.instance.name, Labels: e.Labels(), Replicas: 1, PodConfig: podConfig, diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go index 681a76eb..878c7136 100644 --- a/pkg/instance/instance.go +++ b/pkg/instance/instance.go @@ -10,7 +10,6 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "github.com/celestiaorg/knuu/pkg/k8s" - "github.com/celestiaorg/knuu/pkg/names" "github.com/celestiaorg/knuu/pkg/system" ) @@ -24,7 +23,7 @@ const ( // Instance represents a instance type Instance struct { - system.SystemDependencies + *system.SystemDependencies resources *resources network *network @@ -36,7 +35,6 @@ type Instance struct { sidecars *sidecars name string - k8sName string state InstanceState instanceType InstanceType @@ -45,21 +43,17 @@ type Instance struct { parentInstance *Instance } -func New(name string, sysDeps system.SystemDependencies) (*Instance, error) { - k8sName, err := names.NewRandomK8(name) - if err != nil { - return nil, ErrGeneratingK8sName.WithParams(name).Wrap(err) - } - - // Create the instance +func New(name string, sysDeps *system.SystemDependencies) (*Instance, error) { i := &Instance{ - name: name, - k8sName: k8sName, state: StateNone, instanceType: BasicInstance, SystemDependencies: sysDeps, } + if err := i.SetName(name); err != nil { + return nil, err + } + i.build = &build{ instance: i, command: make([]string, 0), @@ -113,8 +107,20 @@ func (i *Instance) Name() string { return i.name } -func (i *Instance) K8sName() string { - return i.k8sName +func (i *Instance) SetName(name string) error { + name = k8s.SanitizeName(name) + if i.SystemDependencies.HasInstanceName(name) { + return ErrInstanceNameAlreadyExists.WithParams(name) + } + i.SystemDependencies.AddInstanceName(name) + + if i.name != "" { + // Remove the old name from the system dependencies + // So someone else can use it if they want + i.SystemDependencies.RemoveInstanceName(i.name) + } + i.name = name + return nil } func (i *Instance) State() InstanceState { @@ -125,23 +131,9 @@ func (i *Instance) SetInstanceType(instanceType InstanceType) { i.instanceType = instanceType } -// Clone creates a clone of the instance -// This function can only be called in the state 'Committed' -// When cloning an instance that is a sidecar, the clone will be not a sidecar -// When cloning an instance with sidecars, the sidecars will be cloned as well -func (i *Instance) Clone() (*Instance, error) { - if !i.IsInState(StateCommitted) { - return nil, ErrCloningNotAllowed.WithParams(i.state.String()) - } - - newK8sName, err := names.NewRandomK8(i.name) - if err != nil { - return nil, ErrGeneratingK8sName.WithParams(i.name).Wrap(err) - } - // Create a new instance with the same attributes as the original instance - ins := i.CloneWithSuffix("") - ins.k8sName = newK8sName - return ins, nil +// CloneWithSuffix clones the instance with a suffix +func (i *Instance) CloneWithSuffix(suffix string) (*Instance, error) { + return i.CloneWithName(i.name + "-" + suffix) } // CloneWithName creates a clone of the instance with a given name @@ -149,26 +141,12 @@ func (i *Instance) Clone() (*Instance, error) { // When cloning an instance that is a sidecar, the clone will be not a sidecar // When cloning an instance with sidecars, the sidecars will be cloned as well func (i *Instance) CloneWithName(name string) (*Instance, error) { - if !i.IsInState(StateCommitted) { - return nil, ErrCloningNotAllowedForSidecar.WithParams(i.state.String()) - } - - newK8sName, err := names.NewRandomK8(name) + clonedSidecars, err := i.sidecars.clone(name) if err != nil { - return nil, ErrGeneratingK8sNameForSidecar.WithParams(name).Wrap(err) + return nil, err } - // Create a new instance with the same attributes as the original instance - ins := i.CloneWithSuffix("") - ins.name = name - ins.k8sName = newK8sName - return ins, nil -} -// cloneWithSuffix clones the instance with a suffix -func (i *Instance) CloneWithSuffix(suffix string) *Instance { newInstance := &Instance{ - name: i.name + "-" + suffix, - k8sName: i.k8sName + "-" + suffix, SystemDependencies: i.SystemDependencies, build: i.build.clone(), @@ -178,12 +156,16 @@ func (i *Instance) CloneWithSuffix(suffix string) *Instance { storage: i.storage.clone(), monitoring: i.monitoring.clone(), security: i.security.clone(), - sidecars: i.sidecars.cloneWithSuffix(suffix), + sidecars: clonedSidecars, state: i.state, instanceType: i.instanceType, } + if err := newInstance.SetName(name); err != nil { + return nil, err + } + // Need to set all the parent references to the newly created instance newInstance.sidecars.instance = newInstance newInstance.security.instance = newInstance @@ -194,5 +176,5 @@ func (i *Instance) CloneWithSuffix(suffix string) *Instance { newInstance.resources.instance = newInstance newInstance.build.instance = newInstance - return newInstance + return newInstance, nil } diff --git a/pkg/instance/network.go b/pkg/instance/network.go index 10dd74f2..266ec338 100644 --- a/pkg/instance/network.go +++ b/pkg/instance/network.go @@ -62,9 +62,9 @@ func (n *network) PortForwardTCP(ctx context.Context, port int) (int, error) { } // Forward the port - pod, err := n.instance.K8sClient.GetFirstPodFromReplicaSet(ctx, n.instance.k8sName) + pod, err := n.instance.K8sClient.GetFirstPodFromReplicaSet(ctx, n.instance.name) if err != nil { - return -1, ErrGettingPodFromReplicaSet.WithParams(n.instance.k8sName).Wrap(err) + return -1, ErrGettingPodFromReplicaSet.WithParams(n.instance.name).Wrap(err) } for attempt := 1; attempt <= maxRetries; attempt++ { @@ -126,22 +126,22 @@ func (n *network) GetIP(ctx context.Context) (string, error) { return n.kubernetesService.Spec.ClusterIP, nil } // If not, proceed with the existing logic to deploy the service and get the IP - svc, err := n.instance.K8sClient.GetService(ctx, n.instance.k8sName) + svc, err := n.instance.K8sClient.GetService(ctx, n.instance.name) if err != nil || svc == nil { // Service does not exist, so we need to deploy it err := n.deployService(ctx, n.portsTCP, n.portsUDP) if err != nil { - return "", ErrDeployingServiceForInstance.WithParams(n.instance.k8sName).Wrap(err) + return "", ErrDeployingServiceForInstance.WithParams(n.instance.name).Wrap(err) } - svc, err = n.instance.K8sClient.GetService(ctx, n.instance.k8sName) + svc, err = n.instance.K8sClient.GetService(ctx, n.instance.name) if err != nil { - return "", ErrGettingServiceForInstance.WithParams(n.instance.k8sName).Wrap(err) + return "", ErrGettingServiceForInstance.WithParams(n.instance.name).Wrap(err) } } ip := svc.Spec.ClusterIP if ip == "" { - return "", ErrGettingServiceIP.WithParams(n.instance.k8sName) + return "", ErrGettingServiceIP.WithParams(n.instance.name) } // Update i.kubernetesService for future reference @@ -153,18 +153,18 @@ func (n *network) GetIP(ctx context.Context) (string, error) { func (n *network) deployService(ctx context.Context, portsTCP, portsUDP []int) error { // a sidecar instance should use the parent instance's service if n.instance.sidecars.IsSidecar() { - return ErrDeployingServiceForSidecar.WithParams(n.instance.k8sName) + return ErrDeployingServiceForSidecar.WithParams(n.instance.name) } var ( - serviceName = n.instance.k8sName + serviceName = n.instance.name labels = n.instance.execution.Labels() labelSelectors = labels ) srv, err := n.instance.K8sClient.CreateService(ctx, serviceName, labels, labelSelectors, portsTCP, portsUDP) if err != nil { - return ErrDeployingService.WithParams(n.instance.k8sName).Wrap(err) + return ErrDeployingService.WithParams(n.instance.name).Wrap(err) } n.kubernetesService = srv n.instance.Logger.WithFields(logrus.Fields{ @@ -178,11 +178,11 @@ func (n *network) deployService(ctx context.Context, portsTCP, portsUDP []int) e func (n *network) patchService(ctx context.Context, portsTCP, portsUDP []int) error { // a sidecar instance should use the parent instance's service if n.instance.sidecars.IsSidecar() { - return ErrPatchingServiceForSidecar.WithParams(n.instance.k8sName) + return ErrPatchingServiceForSidecar.WithParams(n.instance.name) } var ( - serviceName = n.instance.k8sName + serviceName = n.instance.name labels = n.instance.execution.Labels() labelSelectors = labels ) @@ -201,7 +201,7 @@ func (n *network) patchService(ctx context.Context, portsTCP, portsUDP []int) er // destroyService destroys the service for the instance func (n *network) destroyService(ctx context.Context) error { - return n.instance.K8sClient.DeleteService(ctx, n.instance.k8sName) + return n.instance.K8sClient.DeleteService(ctx, n.instance.name) } // isTCPPortRegistered returns true if the given port is registered @@ -241,9 +241,9 @@ func (n *network) Disable(ctx context.Context) error { return ErrDisablingNetworkNotAllowed.WithParams(n.instance.state.String()) } - err := n.instance.K8sClient.CreateNetworkPolicy(ctx, n.instance.k8sName, n.instance.execution.Labels(), nil, nil) + err := n.instance.K8sClient.CreateNetworkPolicy(ctx, n.instance.name, n.instance.execution.Labels(), nil, nil) if err != nil { - return ErrDisablingNetwork.WithParams(n.instance.k8sName).Wrap(err) + return ErrDisablingNetwork.WithParams(n.instance.name).Wrap(err) } return nil } @@ -255,9 +255,9 @@ func (n *network) Enable(ctx context.Context) error { return ErrEnablingNetworkNotAllowed.WithParams(n.instance.state.String()) } - err := n.instance.K8sClient.DeleteNetworkPolicy(ctx, n.instance.k8sName) + err := n.instance.K8sClient.DeleteNetworkPolicy(ctx, n.instance.name) if err != nil { - return ErrEnablingNetwork.WithParams(n.instance.k8sName).Wrap(err) + return ErrEnablingNetwork.WithParams(n.instance.name).Wrap(err) } return nil } @@ -269,7 +269,7 @@ func (n *network) IsDisabled(ctx context.Context) (bool, error) { return false, ErrCheckingIfNetworkDisabledNotAllowed.WithParams(n.instance.state.String()) } - return n.instance.K8sClient.NetworkPolicyExists(ctx, n.instance.k8sName), nil + return n.instance.K8sClient.NetworkPolicyExists(ctx, n.instance.name), nil } // deployService deploys the service for the instance @@ -279,16 +279,17 @@ func (n *network) deployOrPatchService(ctx context.Context, portsTCP, portsUDP [ } n.instance.Logger.WithField("instance", n.instance.name).Debug("ports not empty, deploying service") - svc, _ := n.instance.K8sClient.GetService(ctx, n.instance.k8sName) + svc, _ := n.instance.K8sClient.GetService(ctx, n.instance.name) + // TODO: check the error and deploy the service if it does not exist if svc == nil { if err := n.deployService(ctx, portsTCP, portsUDP); err != nil { - return ErrDeployingServiceForInstance.WithParams(n.instance.k8sName).Wrap(err) + return ErrDeployingServiceForInstance.WithParams(n.instance.name).Wrap(err) } return nil } if err := n.patchService(ctx, portsTCP, portsUDP); err != nil { - return ErrPatchingServiceForInstance.WithParams(n.instance.k8sName).Wrap(err) + return ErrPatchingServiceForInstance.WithParams(n.instance.name).Wrap(err) } return nil } @@ -296,16 +297,16 @@ func (n *network) deployOrPatchService(ctx context.Context, portsTCP, portsUDP [ func (n *network) enableIfDisabled(ctx context.Context) error { disableNetwork, err := n.IsDisabled(ctx) if err != nil { - n.instance.Logger.WithError(err).WithField("instance", n.instance.k8sName).Error("error checking network status for instance") - return ErrCheckingNetworkStatusForInstance.WithParams(n.instance.k8sName).Wrap(err) + n.instance.Logger.WithError(err).WithField("instance", n.instance.name).Error("error checking network status for instance") + return ErrCheckingNetworkStatusForInstance.WithParams(n.instance.name).Wrap(err) } if !disableNetwork { return nil } if err := n.Enable(ctx); err != nil { - n.instance.Logger.WithError(err).WithField("instance", n.instance.k8sName).Error("error enabling network for instance") - return ErrEnablingNetworkForInstance.WithParams(n.instance.k8sName).Wrap(err) + n.instance.Logger.WithError(err).WithField("instance", n.instance.name).Error("error enabling network for instance") + return ErrEnablingNetworkForInstance.WithParams(n.instance.name).Wrap(err) } return nil } diff --git a/pkg/instance/proxy.go b/pkg/instance/proxy.go index 378b009e..79a3760d 100644 --- a/pkg/instance/proxy.go +++ b/pkg/instance/proxy.go @@ -15,12 +15,12 @@ func (n *network) AddHost(ctx context.Context, port int) (host string, err error return "", ErrProxyNotInitialized } - serviceName := n.instance.k8sName + serviceName := n.instance.name if n.instance.sidecars.IsSidecar() { // The service is created for the main instance and // named after it which will be the parent instance for sidecars, // so we need to use the parent instance's service name. - serviceName = n.instance.parentInstance.k8sName + serviceName = n.instance.parentInstance.name } prefix := fmt.Sprintf("%s-%d", serviceName, port) diff --git a/pkg/instance/resources.go b/pkg/instance/resources.go index 4b633f7d..1ca6a551 100644 --- a/pkg/instance/resources.go +++ b/pkg/instance/resources.go @@ -60,7 +60,7 @@ func (r *resources) CreateCustomResource(ctx context.Context, gvr *schema.GroupV return ErrCustomResourceDefinitionDoesNotExist.WithParams(gvr.Resource) } - return r.instance.K8sClient.CreateCustomResource(ctx, r.instance.k8sName, gvr, obj) + return r.instance.K8sClient.CreateCustomResource(ctx, r.instance.name, gvr, obj) } // CustomResourceDefinitionExists checks if the custom resource definition exists @@ -86,7 +86,7 @@ func (r *resources) deployResources(ctx context.Context) error { func (r *resources) deployStorage(ctx context.Context) error { if len(r.instance.storage.volumes) != 0 { if err := r.instance.storage.deployVolume(ctx); err != nil { - return ErrDeployingVolumeForInstance.WithParams(r.instance.k8sName).Wrap(err) + return ErrDeployingVolumeForInstance.WithParams(r.instance.name).Wrap(err) } } if len(r.instance.storage.files) == 0 { @@ -94,7 +94,7 @@ func (r *resources) deployStorage(ctx context.Context) error { } if err := r.instance.storage.deployFiles(ctx); err != nil { - return ErrDeployingFilesForInstance.WithParams(r.instance.k8sName).Wrap(err) + return ErrDeployingFilesForInstance.WithParams(r.instance.name).Wrap(err) } return nil } @@ -118,20 +118,20 @@ func (r *resources) deployService(ctx context.Context) error { func (r *resources) destroyResources(ctx context.Context) error { if len(r.instance.storage.volumes) != 0 { if err := r.instance.storage.destroyVolume(ctx); err != nil { - return ErrDestroyingVolumeForInstance.WithParams(r.instance.k8sName).Wrap(err) + return ErrDestroyingVolumeForInstance.WithParams(r.instance.name).Wrap(err) } } if len(r.instance.storage.files) != 0 { err := r.instance.storage.destroyFiles(ctx) if err != nil { - return ErrDestroyingFilesForInstance.WithParams(r.instance.k8sName).Wrap(err) + return ErrDestroyingFilesForInstance.WithParams(r.instance.name).Wrap(err) } } if r.instance.network.kubernetesService != nil { err := r.instance.network.destroyService(ctx) if err != nil { - return ErrDestroyingServiceForInstance.WithParams(r.instance.k8sName).Wrap(err) + return ErrDestroyingServiceForInstance.WithParams(r.instance.name).Wrap(err) } } @@ -139,7 +139,7 @@ func (r *resources) destroyResources(ctx context.Context) error { if !r.instance.sidecars.IsSidecar() { // enable network when network is disabled if err := r.instance.network.enableIfDisabled(ctx); err != nil { - return ErrEnablingNetworkForInstance.WithParams(r.instance.k8sName).Wrap(err) + return ErrEnablingNetworkForInstance.WithParams(r.instance.name).Wrap(err) } } diff --git a/pkg/instance/sidecars.go b/pkg/instance/sidecars.go index bd0d3db5..c3e1226e 100644 --- a/pkg/instance/sidecars.go +++ b/pkg/instance/sidecars.go @@ -9,10 +9,12 @@ import ( ) type SidecarManager interface { - Initialize(ctx context.Context, sysDeps system.SystemDependencies) error + // namePrefix is the name of the instance that is the parent of the sidecar + // it is used to set the name of the sidecar to avoid name collisions + Initialize(ctx context.Context, namePrefix string, sysDeps *system.SystemDependencies) error Instance() *Instance PreStart(ctx context.Context) error - CloneWithSuffix(suffix string) SidecarManager + Clone(namePrefix string) (SidecarManager, error) } type sidecars struct { @@ -43,7 +45,7 @@ func (s *sidecars) Add(ctx context.Context, sc SidecarManager) error { return ErrAddingSidecarNotAllowed.WithParams(s.instance.state.String()) } - if err := sc.Initialize(ctx, s.instance.SystemDependencies); err != nil { + if err := sc.Initialize(ctx, s.instance.Name(), s.instance.SystemDependencies); err != nil { return ErrInitializingSidecar.WithParams(s.instance.name).Wrap(err) } @@ -82,7 +84,7 @@ func (s *sidecars) verifySidecarsStates() error { func (s *sidecars) applyFunctionToSidecars(fn func(sc SidecarManager) error) error { for _, i := range s.sidecars { if err := fn(i); err != nil { - return ErrApplyingFunctionToSidecar.WithParams(i.Instance().k8sName).Wrap(err) + return ErrApplyingFunctionToSidecar.WithParams(i.Instance().name).Wrap(err) } } return nil @@ -97,12 +99,16 @@ func (s *sidecars) setStateForSidecars(state InstanceState) { }) } -func (s *sidecars) cloneWithSuffix(suffix string) *sidecars { +func (s *sidecars) clone(namePrefix string) (*sidecars, error) { clonedSidecars := make([]SidecarManager, len(s.sidecars)) for i, sc := range s.sidecars { - clonedSidecars[i] = sc.CloneWithSuffix(suffix) + cloned, err := sc.Clone(namePrefix) + if err != nil { + return nil, err + } + clonedSidecars[i] = cloned } return &sidecars{ sidecars: clonedSidecars, - } + }, nil } diff --git a/pkg/instance/storage.go b/pkg/instance/storage.go index 42d3d3ff..b24d501b 100644 --- a/pkg/instance/storage.go +++ b/pkg/instance/storage.go @@ -307,7 +307,7 @@ func (s *storage) deployVolume(ctx context.Context) error { for _, volume := range s.volumes { totalSize.Add(volume.Size) } - s.instance.K8sClient.CreatePersistentVolumeClaim(ctx, s.instance.k8sName, s.instance.execution.Labels(), totalSize) + s.instance.K8sClient.CreatePersistentVolumeClaim(ctx, s.instance.name, s.instance.execution.Labels(), totalSize) s.instance.Logger.WithFields(logrus.Fields{ "total_size": totalSize.String(), "instance": s.instance.name, @@ -318,7 +318,7 @@ func (s *storage) deployVolume(ctx context.Context) error { // destroyVolume destroys the volume for the instance func (s *storage) destroyVolume(ctx context.Context) error { - err := s.instance.K8sClient.DeletePersistentVolumeClaim(ctx, s.instance.k8sName) + err := s.instance.K8sClient.DeletePersistentVolumeClaim(ctx, s.instance.name) if err != nil { return ErrFailedToDeletePersistentVolumeClaim.Wrap(err) } @@ -352,23 +352,23 @@ func (s *storage) deployFiles(ctx context.Context) error { } // create configmap - _, err := s.instance.K8sClient.CreateConfigMap(ctx, s.instance.k8sName, s.instance.execution.Labels(), data) + _, err := s.instance.K8sClient.CreateConfigMap(ctx, s.instance.name, s.instance.execution.Labels(), data) if err != nil { return ErrFailedToCreateConfigMap.Wrap(err) } - s.instance.Logger.WithField("configmap", s.instance.k8sName).Debug("deployed configmap") + s.instance.Logger.WithField("configmap", s.instance.name).Debug("deployed configmap") return nil } // destroyFiles destroys the files for the instance func (s *storage) destroyFiles(ctx context.Context) error { - if err := s.instance.K8sClient.DeleteConfigMap(ctx, s.instance.k8sName); err != nil { + if err := s.instance.K8sClient.DeleteConfigMap(ctx, s.instance.name); err != nil { return ErrFailedToDeleteConfigMap.Wrap(err) } - s.instance.Logger.WithField("configmap", s.instance.k8sName).Debug("destroyed configmap") + s.instance.Logger.WithField("configmap", s.instance.name).Debug("destroyed configmap") return nil } @@ -403,7 +403,7 @@ func (s *storage) readFileFromImage(ctx context.Context, filePath string) ([]byt } defer func() { if err := ti.execution.Destroy(ctx); err != nil { - ti.Logger.Errorf("failed to destroy tmp instance %s: %v", ti.k8sName, err) + ti.Logger.Errorf("failed to destroy tmp instance %s: %v", ti.name, err) } }() diff --git a/pkg/knuu/instance.go b/pkg/knuu/instance.go index c9e6e752..5a086588 100644 --- a/pkg/knuu/instance.go +++ b/pkg/knuu/instance.go @@ -10,6 +10,6 @@ func (k *Knuu) NewInstance(name string) (*instance.Instance, error) { return instance.New(name, k.SystemDependencies) } -func (k *Knuu) NewPreloader() (*preloader.Preloader, error) { - return preloader.New(k.SystemDependencies) +func (k *Knuu) NewPreloader(name string) (*preloader.Preloader, error) { + return preloader.New(name, k.SystemDependencies) } diff --git a/pkg/knuu/knuu.go b/pkg/knuu/knuu.go index edebef96..3d2fcd94 100644 --- a/pkg/knuu/knuu.go +++ b/pkg/knuu/knuu.go @@ -33,7 +33,7 @@ const ( ) type Knuu struct { - system.SystemDependencies + *system.SystemDependencies timeout time.Duration } @@ -53,7 +53,7 @@ func New(ctx context.Context, opts Options) (*Knuu, error) { } k := &Knuu{ - SystemDependencies: system.SystemDependencies{ + SystemDependencies: &system.SystemDependencies{ K8sClient: opts.K8sClient, MinioClient: opts.MinioClient, ImageBuilder: opts.ImageBuilder, diff --git a/pkg/knuu/knuu_test.go b/pkg/knuu/knuu_test.go index fe99fca8..03f55d65 100644 --- a/pkg/knuu/knuu_test.go +++ b/pkg/knuu/knuu_test.go @@ -98,7 +98,6 @@ func TestNew(t *testing.T) { { name: "With custom Logger", options: Options{ - Scope: "test", Logger: &logrus.Logger{}, }, expectedError: nil, @@ -110,7 +109,6 @@ func TestNew(t *testing.T) { { name: "With custom Timeout", options: Options{ - Scope: "test", Timeout: 30 * time.Minute, }, expectedError: nil, @@ -122,7 +120,6 @@ func TestNew(t *testing.T) { { name: "With custom Image Builder", options: Options{ - Scope: "test", ImageBuilder: &kaniko.Kaniko{}, }, expectedError: nil, diff --git a/pkg/preloader/errors.go b/pkg/preloader/errors.go index 08beb826..0b16a3c4 100644 --- a/pkg/preloader/errors.go +++ b/pkg/preloader/errors.go @@ -7,5 +7,5 @@ import ( type Error = errors.Error var ( - ErrGeneratingK8sNameForPreloader = errors.New("GeneratingK8sNameForPreloader", "error generating k8s name for preloader") + ErrPreloaderNameAlreadyExists = errors.New("PreloaderNameAlreadyExists", "preloader name '%s' already exists") ) diff --git a/pkg/preloader/preloader.go b/pkg/preloader/preloader.go index b60f782b..2b807178 100644 --- a/pkg/preloader/preloader.go +++ b/pkg/preloader/preloader.go @@ -6,7 +6,7 @@ import ( v1 "k8s.io/api/core/v1" - "github.com/celestiaorg/knuu/pkg/names" + "github.com/celestiaorg/knuu/pkg/k8s" "github.com/celestiaorg/knuu/pkg/system" ) @@ -30,19 +30,21 @@ const ( // A preloader makes sure that the images are preloaded before the test suite starts. // Hint: If you use a Preloader per test suite, you can save resources type Preloader struct { - K8sName string `json:"k8sName"` - Images []string `json:"images"` - system.SystemDependencies + Name string `json:"name"` + Images []string `json:"images"` + *system.SystemDependencies } // New creates a new preloader -func New(sysDeps system.SystemDependencies) (*Preloader, error) { - k8sName, err := names.NewRandomK8(preloaderName) - if err != nil { - return nil, ErrGeneratingK8sNameForPreloader.Wrap(err) +func New(name string, sysDeps *system.SystemDependencies) (*Preloader, error) { + name = k8s.SanitizeName(name) + if sysDeps.HasInstanceName(name) { + return nil, ErrPreloaderNameAlreadyExists.WithParams(name) } + sysDeps.AddInstanceName(name) + return &Preloader{ - K8sName: k8sName, + Name: name, Images: []string{}, SystemDependencies: sysDeps, }, nil @@ -87,7 +89,7 @@ func (p *Preloader) EmptyImages(ctx context.Context) error { func (p *Preloader) preloadImages(ctx context.Context) error { // delete the daemonset if no images are preloaded if len(p.Images) == 0 { - return p.K8sClient.DeleteDaemonSet(ctx, p.K8sName) + return p.K8sClient.DeleteDaemonSet(ctx, p.Name) } var initContainers []v1.Container @@ -111,28 +113,28 @@ func (p *Preloader) preloadImages(ctx context.Context) error { }) labels := map[string]string{ - labelApp: p.K8sName, + labelApp: p.Name, labelManagedBy: managedByLabel, labelScope: p.Scope, labelTestStarted: p.StartTime, } - exists, err := p.K8sClient.DaemonSetExists(ctx, p.K8sName) + exists, err := p.K8sClient.DaemonSetExists(ctx, p.Name) if err != nil { return err } // update the daemonset if it already exists if exists { - _, err = p.K8sClient.UpdateDaemonSet(ctx, p.K8sName, labels, initContainers, containers) + _, err = p.K8sClient.UpdateDaemonSet(ctx, p.Name, labels, initContainers, containers) return err } // create the daemonset if it doesn't exist - _, err = p.K8sClient.CreateDaemonSet(ctx, p.K8sName, labels, initContainers, containers) + _, err = p.K8sClient.CreateDaemonSet(ctx, p.Name, labels, initContainers, containers) return err } func (p *Preloader) Cleanup(ctx context.Context) error { - return p.K8sClient.DeleteDaemonSet(ctx, p.K8sName) + return p.K8sClient.DeleteDaemonSet(ctx, p.Name) } diff --git a/pkg/sidecars/netshaper/netshaper.go b/pkg/sidecars/netshaper/netshaper.go index b2b50129..a280e070 100644 --- a/pkg/sidecars/netshaper/netshaper.go +++ b/pkg/sidecars/netshaper/netshaper.go @@ -42,9 +42,9 @@ func New() *NetShaper { // Initialize initializes the BitTwister sidecar // and it is called once the instance.AddSidecar is called -func (bt *NetShaper) Initialize(ctx context.Context, sysDeps system.SystemDependencies) error { +func (bt *NetShaper) Initialize(ctx context.Context, namePrefix string, sysDeps *system.SystemDependencies) error { var err error - bt.instance, err = instance.New(instanceName, sysDeps) + bt.instance, err = instance.New(namePrefix+"-"+instanceName, sysDeps) if err != nil { return ErrCreatingBitTwisterInstance.Wrap(err) } @@ -99,11 +99,15 @@ func (bt *NetShaper) Instance() *instance.Instance { return bt.instance } -func (bt *NetShaper) CloneWithSuffix(suffix string) instance.SidecarManager { +func (bt *NetShaper) Clone(namePrefix string) (instance.SidecarManager, error) { + clone, err := bt.instance.CloneWithName(namePrefix + "-" + instanceName) + if err != nil { + return nil, err + } return &NetShaper{ - instance: bt.instance.CloneWithSuffix(suffix), + instance: clone, port: bt.port, image: bt.image, networkInterface: bt.networkInterface, - } + }, nil } diff --git a/pkg/sidecars/netshaper/netshaper_test.go b/pkg/sidecars/netshaper/netshaper_test.go index 8182643c..f6b3e381 100644 --- a/pkg/sidecars/netshaper/netshaper_test.go +++ b/pkg/sidecars/netshaper/netshaper_test.go @@ -18,14 +18,14 @@ type TestSuite struct { suite.Suite bt *NetShaper ctx context.Context - sysDeps system.SystemDependencies + sysDeps *system.SystemDependencies mockServer *httptest.Server } func (s *TestSuite) SetupTest() { s.bt = New() s.ctx = context.Background() - s.sysDeps = system.SystemDependencies{ + s.sysDeps = &system.SystemDependencies{ Logger: logrus.New(), } @@ -64,7 +64,7 @@ func (s *TestSuite) TestNew() { } func (s *TestSuite) TestInitialize() { - err := s.bt.Initialize(s.ctx, s.sysDeps) + err := s.bt.Initialize(s.ctx, "test-init", s.sysDeps) s.Require().NoError(err) s.Assert().NotNil(s.bt.Instance()) s.Assert().Equal(DefaultImage, s.bt.Instance().Build().ImageName()) @@ -75,12 +75,14 @@ func (s *TestSuite) TestPreStart() { s.T().Skip("skipping as it is tested in e2e tests") } -func (s *TestSuite) TestCloneWithSuffix() { - err := s.bt.Initialize(s.ctx, s.sysDeps) +func (s *TestSuite) TestClone() { + err := s.bt.Initialize(s.ctx, "test-clone", s.sysDeps) s.Require().NoError(err) s.Require().NotNil(s.bt.instance, "Instance should be initialized before cloning") - clone := s.bt.CloneWithSuffix("test") + clonePrefixName := "test-clone-prefix" + clone, err := s.bt.Clone(clonePrefixName) + s.Require().NoError(err) s.Assert().NotNil(clone) clonedBt, ok := clone.(*NetShaper) @@ -93,13 +95,13 @@ func (s *TestSuite) TestCloneWithSuffix() { s.Assert().NotNil(clonedBt.instance, "Cloned instance should not be nil") s.Assert().NotEqual(s.bt.instance, clonedBt.instance, "Cloned instance should be a new object") s.Assert().Equal(s.bt.instance.Build().ImageName(), clonedBt.instance.Build().ImageName()) - s.Assert().Equal(s.bt.instance.Name()+"-test", clonedBt.instance.Name()) + s.Assert().Equal(clonePrefixName+"-"+instanceName, clonedBt.instance.Name()) clonedBt.SetPort(9090) s.Assert().NotEqual(s.bt.port, clonedBt.port) } -func (s *TestSuite) TestCloneWithSuffixWithCustomValues() { - err := s.bt.Initialize(s.ctx, s.sysDeps) +func (s *TestSuite) TestCloneWithCustomValues() { + err := s.bt.Initialize(s.ctx, "test-clone-custom", s.sysDeps) s.Require().NoError(err) s.Require().NotNil(s.bt.instance, "Instance should be initialized before cloning") @@ -107,7 +109,9 @@ func (s *TestSuite) TestCloneWithSuffixWithCustomValues() { s.bt.SetImage("nginx") s.bt.SetNetworkInterface("eth0") - clone := s.bt.CloneWithSuffix("test") + clonePrefixName := "test-clone-custom-prefix" + clone, err := s.bt.Clone(clonePrefixName) + s.Require().NoError(err) s.Assert().NotNil(clone) clonedBt, ok := clone.(*NetShaper) @@ -120,7 +124,7 @@ func (s *TestSuite) TestCloneWithSuffixWithCustomValues() { s.Assert().NotNil(clonedBt.instance, "Cloned instance should not be nil") s.Assert().NotEqual(s.bt.instance, clonedBt.instance, "Cloned instance should be a new object") s.Assert().Equal(s.bt.instance.Build().ImageName(), clonedBt.instance.Build().ImageName()) - s.Assert().Equal(s.bt.instance.Name()+"-test", clonedBt.instance.Name()) + s.Assert().Equal(clonePrefixName+"-"+instanceName, clonedBt.instance.Name()) clonedBt.SetPort(9090) s.Assert().NotEqual(s.bt.port, clonedBt.port) diff --git a/pkg/sidecars/observability/obsy.go b/pkg/sidecars/observability/obsy.go index f332d338..dc66f0ad 100644 --- a/pkg/sidecars/observability/obsy.go +++ b/pkg/sidecars/observability/obsy.go @@ -86,9 +86,9 @@ func New() *Obsy { } } -func (o *Obsy) Initialize(ctx context.Context, sysDeps system.SystemDependencies) error { +func (o *Obsy) Initialize(ctx context.Context, namePrefix string, sysDeps *system.SystemDependencies) error { var err error - o.instance, err = instance.New(otelAgentName, sysDeps) + o.instance, err = instance.New(namePrefix+"-"+otelAgentName, sysDeps) if err != nil { return ErrCreatingOtelAgentInstance.Wrap(err) } @@ -149,10 +149,14 @@ func (o *Obsy) Instance() *instance.Instance { return o.instance } -func (o *Obsy) CloneWithSuffix(suffix string) instance.SidecarManager { +func (o *Obsy) Clone(namePrefix string) (instance.SidecarManager, error) { + clone, err := o.instance.CloneWithName(namePrefix + "-" + otelAgentName) + if err != nil { + return nil, err + } conf := *o.obsyConfig return &Obsy{ - instance: o.instance.CloneWithSuffix(suffix), + instance: clone, obsyConfig: &conf, - } + }, nil } diff --git a/pkg/sidecars/observability/obsy_test.go b/pkg/sidecars/observability/obsy_test.go index 3ec6f4f5..f58238e8 100644 --- a/pkg/sidecars/observability/obsy_test.go +++ b/pkg/sidecars/observability/obsy_test.go @@ -13,7 +13,7 @@ import ( type TestSuite struct { suite.Suite - sysDeps system.SystemDependencies + sysDeps *system.SystemDependencies } func TestObsyTestSuite(t *testing.T) { @@ -30,7 +30,7 @@ func (m *mockK8sCli) Namespace() string { } func (s *TestSuite) SetupTest() { - s.sysDeps = system.SystemDependencies{ + s.sysDeps = &system.SystemDependencies{ K8sClient: &mockK8sCli{ namespace: "test", KubeManager: &k8s.Client{}, @@ -47,7 +47,7 @@ func (s *TestSuite) TestNew() { func (s *TestSuite) TestInitialize() { o := New() - err := o.Initialize(context.Background(), s.sysDeps) + err := o.Initialize(context.Background(), "test-init", s.sysDeps) s.Require().NoError(err) s.Assert().NotNil(o.Instance()) s.Assert().True(o.Instance().Sidecars().IsSidecar()) @@ -57,17 +57,20 @@ func (s *TestSuite) TestPreStart() { s.T().Skip("skipping as it is tested in e2e tests") } -func (s *TestSuite) TestCloneWithSuffix() { +func (s *TestSuite) TestClone() { o := New() - err := o.Initialize(context.Background(), s.sysDeps) + err := o.Initialize(context.Background(), "test-clone", s.sysDeps) s.Require().NoError(err) - clone := o.CloneWithSuffix("test") + clonePrefixName := "test-clone-prefix" + clone, err := o.Clone(clonePrefixName) + s.Require().NoError(err) s.Assert().NotNil(clone) clonedObsy, ok := clone.(*Obsy) s.Assert().True(ok) s.Assert().Equal(o.obsyConfig, clonedObsy.obsyConfig) + s.Assert().Equal(clonePrefixName+"-"+otelAgentName, clonedObsy.Instance().Name()) } func (s *TestSuite) TestSetters() { diff --git a/pkg/sidecars/tshark/tshark.go b/pkg/sidecars/tshark/tshark.go index c2aa8617..5264ca06 100644 --- a/pkg/sidecars/tshark/tshark.go +++ b/pkg/sidecars/tshark/tshark.go @@ -64,7 +64,7 @@ var ( // Initialize initializes the BitTwister sidecar // and it is called once the instance.AddSidecar is called -func (t *Tshark) Initialize(ctx context.Context, sysDeps system.SystemDependencies) error { +func (t *Tshark) Initialize(ctx context.Context, namePrefix string, sysDeps *system.SystemDependencies) error { if err := t.validateConfig(); err != nil { return err } @@ -74,7 +74,7 @@ func (t *Tshark) Initialize(ctx context.Context, sysDeps system.SystemDependenci } var err error - t.instance, err = instance.New(tsharkCollectorName, sysDeps) + t.instance, err = instance.New(namePrefix+"-"+tsharkCollectorName, sysDeps) if err != nil { return ErrCreatingTsharkCollectorInstance.Wrap(err) } @@ -105,7 +105,7 @@ func (t *Tshark) Initialize(ctx context.Context, sysDeps system.SystemDependenci envStorageRegion: t.S3Region, envStorageBucketName: t.S3Bucket, envStorageKeyPrefix: t.S3KeyPrefix, - envCaptureFileName: t.instance.K8sName() + TsharkCaptureFileExtension, + envCaptureFileName: t.instance.Name() + TsharkCaptureFileExtension, envStorageEndpoint: t.S3Endpoint, envUploadInterval: fmt.Sprintf("%d", int64(t.UploadInterval.Seconds())), envCreateBucket: fmt.Sprintf("%t", t.CreateBucket), @@ -135,9 +135,13 @@ func (t *Tshark) Instance() *instance.Instance { return t.instance } -func (t *Tshark) CloneWithSuffix(suffix string) instance.SidecarManager { +func (t *Tshark) Clone(namePrefix string) (instance.SidecarManager, error) { + clone, err := t.instance.CloneWithName(namePrefix + "-" + tsharkCollectorName) + if err != nil { + return nil, err + } return &Tshark{ - instance: t.instance.CloneWithSuffix(suffix), + instance: clone, VolumeSize: t.VolumeSize, S3AccessKey: t.S3AccessKey, S3SecretKey: t.S3SecretKey, @@ -147,5 +151,5 @@ func (t *Tshark) CloneWithSuffix(suffix string) instance.SidecarManager { S3KeyPrefix: t.S3KeyPrefix, S3Endpoint: t.S3Endpoint, UploadInterval: t.UploadInterval, - } + }, nil } diff --git a/pkg/sidecars/tshark/tshark_test.go b/pkg/sidecars/tshark/tshark_test.go index 94ab1203..e91eadcc 100644 --- a/pkg/sidecars/tshark/tshark_test.go +++ b/pkg/sidecars/tshark/tshark_test.go @@ -83,7 +83,7 @@ func TestTsharkInitialize(t *testing.T) { logrus.New(), ) require.NoError(t, err) - sysDeps := system.SystemDependencies{ + sysDeps := &system.SystemDependencies{ K8sClient: k8sClient, Logger: logger, } @@ -91,7 +91,7 @@ func TestTsharkInitialize(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := tt.config.Initialize(ctx, sysDeps) + err := tt.config.Initialize(ctx, "test-init", sysDeps) if tt.wantErr != nil { require.Error(t, err) @@ -168,8 +168,8 @@ func TestTsharkValidateConfig(t *testing.T) { } } -func TestTsharkCloneWithSuffix(t *testing.T) { - testInstance, err := instance.New("testInstance", system.SystemDependencies{}) +func TestTsharkClone(t *testing.T) { + testInstance, err := instance.New("testInstance", &system.SystemDependencies{}) require.NoError(t, err) tshark := &Tshark{ @@ -185,7 +185,9 @@ func TestTsharkCloneWithSuffix(t *testing.T) { instance: testInstance, } - clone := tshark.CloneWithSuffix("-clone") + clonePrefixName := "test-clone-prefix" + clone, err := tshark.Clone(clonePrefixName) + require.NoError(t, err) assert.Equal(t, tshark.VolumeSize, clone.(*Tshark).VolumeSize) assert.Equal(t, tshark.S3AccessKey, clone.(*Tshark).S3AccessKey) @@ -197,4 +199,5 @@ func TestTsharkCloneWithSuffix(t *testing.T) { assert.Equal(t, tshark.S3Endpoint, clone.(*Tshark).S3Endpoint) assert.Equal(t, tshark.UploadInterval, clone.(*Tshark).UploadInterval) assert.NotEmpty(t, clone.(*Tshark).instance) + assert.Equal(t, clonePrefixName+"-"+tsharkCollectorName, clone.(*Tshark).instance.Name()) } diff --git a/pkg/system/dependencies.go b/pkg/system/dependencies.go index 524ed521..67af0c38 100644 --- a/pkg/system/dependencies.go +++ b/pkg/system/dependencies.go @@ -1,6 +1,8 @@ package system import ( + "sync" + "github.com/sirupsen/logrus" "github.com/celestiaorg/knuu/pkg/builder" @@ -17,4 +19,18 @@ type SystemDependencies struct { Proxy *traefik.Traefik Scope string StartTime string + instancesMap sync.Map +} + +func (s *SystemDependencies) AddInstanceName(name string) { + s.instancesMap.Store(name, struct{}{}) +} + +func (s *SystemDependencies) HasInstanceName(name string) bool { + _, exists := s.instancesMap.Load(name) + return exists +} + +func (s *SystemDependencies) RemoveInstanceName(name string) { + s.instancesMap.Delete(name) }