Ext-container UniquenessCheck fix (#1514)

* Skip the container uniqueness check for ext-containers

* comments added

* add tests

* add package

* add mitting test topo file

clab/config.go

@@ -544,6 +544,9 @@ func (c *CLab) VerifyContainersUniqueness(ctx context.Context) error {
 
 	dups := []string{}
 	for _, n := range c.Nodes {
+		if n.Config().SkipUniquenessCheck {
+			continue
+		}
 		for _, cnt := range containers {
 			if n.Config().LongName == cnt.Names[0] {
 				dups = append(dups, n.Config().LongName)

clab/config_test.go

@@ -5,16 +5,21 @@
 package clab
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"path/filepath"
 	"reflect"
 	"testing"
 
 	"github.com/containers/podman/v4/pkg/util"
+	"github.com/golang/mock/gomock"
 	"github.com/google/go-cmp/cmp"
 	"github.com/srl-labs/containerlab/labels"
+	"github.com/srl-labs/containerlab/mocks"
+	"github.com/srl-labs/containerlab/runtime"
 	"github.com/srl-labs/containerlab/utils"
+	"github.com/stretchr/testify/assert"
 )
 
 func setupTestCase(t *testing.T) func(t *testing.T) {
@@ -482,6 +487,110 @@ func TestVerifyRootNetnsInterfaceUniqueness(t *testing.T) {
 	t.Logf("error: %v", err)
 }
 
+func TestVerifyContainersUniqueness(t *testing.T) {
+	tests := map[string]struct {
+		mockResult struct {
+			c []runtime.GenericContainer
+			e error
+		}
+		topo      string
+		wantError bool
+	}{
+		"no dups": {
+			mockResult: struct {
+				c []runtime.GenericContainer
+				e error
+			}{
+				c: []runtime.GenericContainer{
+					{
+						Names:  []string{"some node"},
+						Labels: map[string]string{},
+					},
+					{
+						Names:  []string{"some other node"},
+						Labels: map[string]string{},
+					},
+				},
+				e: nil,
+			},
+			topo:      "test_data/topo1.yml",
+			wantError: false,
+		},
+		"dups": {
+			mockResult: struct {
+				c []runtime.GenericContainer
+				e error
+			}{
+				c: []runtime.GenericContainer{
+					{
+						Names:  []string{"clab-topo1-node1"},
+						Labels: map[string]string{},
+					},
+					{
+						Names:  []string{"somenode"},
+						Labels: map[string]string{},
+					},
+				},
+				e: nil,
+			},
+			wantError: true,
+			topo:      "test_data/topo1.yml",
+		},
+		"ext-container": {
+			mockResult: struct {
+				c []runtime.GenericContainer
+				e error
+			}{
+				c: []runtime.GenericContainer{
+					{
+						Names:  []string{"node1"},
+						Labels: map[string]string{},
+					},
+					{
+						Names:  []string{"somenode"},
+						Labels: map[string]string{},
+					},
+				},
+				e: nil,
+			},
+			wantError: false,
+			topo:      "test_data/topo11-ext-cont.yaml",
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			// init Runtime Mock
+			ctrl := gomock.NewController(t)
+			rtName := "mock"
+
+			opts := []ClabOption{
+				WithTopoFile(tc.topo, ""),
+			}
+			c, err := NewContainerLab(opts...)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			// set mockRuntime parameters
+			mockRuntime := mocks.NewMockContainerRuntime(ctrl)
+			c.Runtimes[rtName] = mockRuntime
+			c.globalRuntime = rtName
+
+			// prepare runtime result
+			mockRuntime.EXPECT().ListContainers(gomock.Any(), gomock.Any()).AnyTimes().Return(tc.mockResult.c, tc.mockResult.e)
+
+			ctx := context.Background()
+			err = c.VerifyContainersUniqueness(ctx)
+			if tc.wantError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
 func TestEnvFileInit(t *testing.T) {
 	tests := map[string]struct {
 		got  string

clab/test_data/topo11-ext-cont.yaml

@@ -0,0 +1,12 @@
+name: topo11
+topology:
+  nodes:
+    node1:
+      kind: ext-container
+      exec:
+        - ip l
+    node2:
+      kind: linux
+      image: alpine:latest
+
+

go.mod

@@ -36,6 +36,7 @@ require (
 	github.com/scrapli/scrapligo v1.1.10
 	github.com/sirupsen/logrus v1.9.3
 	github.com/spf13/cobra v1.7.0
+	github.com/stretchr/testify v1.8.4
 	github.com/tklauser/numcpus v0.6.1
 	github.com/vishvananda/netlink v1.2.1-beta.2
 	github.com/weaveworks/ignite v0.10.0
@@ -83,6 +84,7 @@ require (
 	github.com/moby/sys/sequential v0.5.0 // indirect
 	github.com/oklog/ulid v1.3.1 // indirect
 	github.com/pkg/sftp v1.13.5 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/sigstore/fulcio v1.3.1 // indirect
 	github.com/sigstore/rekor v1.2.2-0.20230601122533-4c81ff246d12 // indirect
 	github.com/sigstore/sigstore v1.7.1 // indirect

go.sum

@@ -2412,6 +2412,7 @@ github.com/stretchr/testify v1.7.5/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
+github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
 github.com/subosito/gotenv v1.4.1/go.mod h1:ayKnFf/c6rvx/2iiLrJUk1e6plDbT3edrFNGqEflhK0=
 github.com/sylabs/sif/v2 v2.11.5 h1:7ssPH3epSonsTrzbS1YxeJ9KuqAN7ISlSM61a7j/mQM=

nodes/ext_container/ext_container.go

@@ -34,6 +34,9 @@ func (s *extcont) Init(cfg *types.NodeConfig, opts ...nodes.NodeOption) error {
 	for _, o := range opts {
 		o(s)
 	}
+	// Indicate that the pre-deployment UniquenessCheck is to be skipped.
+	// Since we would stop deployment on pre-existing containers.
+	s.Cfg.SkipUniquenessCheck = true
 	return nil
 }
 

types/types.go

@@ -180,10 +180,20 @@ type NodeConfig struct {
 	Memory string  `json:"memory,omitempty"`
 
 	// Extra node parameters
-	Extras               *Extras    `json:"extras,omitempty"`
-	WaitFor              []string   `json:"wait-for,omitempty"`
-	DNS                  *DNSConfig `json:"dns,omitempty"`
+	Extras  *Extras    `json:"extras,omitempty"`
+	WaitFor []string   `json:"wait-for,omitempty"`
+	DNS     *DNSConfig `json:"dns,omitempty"`
+
+	// Kind parameters
+	////////////////////
+	// IsRootNamespaceBased flag indicates that a certain nodes network
+	// namespace (usually based on the kind) is the root network namespace
 	IsRootNamespaceBased bool
+	// SkipUniquenessCheck prevents the pre-deploy uniqueness check, where
+	// we check, that the given node name is not already present on the host.
+	// Introduced to prevent the check from running with ext-containers, since
+	// they should be present by definition.
+	SkipUniquenessCheck bool
 }
 
 func DisableTxOffload(n *NodeConfig) error {