From ba86eca2c2cc521c0b526cbe41aa7b32cfe7023a Mon Sep 17 00:00:00 2001 From: Markus Vahlenkamp Date: Mon, 14 Aug 2023 12:59:00 +0200 Subject: [PATCH] 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 | 3 + clab/config_test.go | 109 +++++++++++++++++++++++++++ clab/test_data/topo11-ext-cont.yaml | 12 +++ go.mod | 2 + go.sum | 1 + nodes/ext_container/ext_container.go | 3 + types/types.go | 16 +++- 7 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 clab/test_data/topo11-ext-cont.yaml diff --git a/clab/config.go b/clab/config.go index 42b8a88f4..3a545b88a 100644 --- a/clab/config.go +++ b/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) diff --git a/clab/config_test.go b/clab/config_test.go index c1ef4d63a..866a0f9a9 100644 --- a/clab/config_test.go +++ b/clab/config_test.go @@ -5,6 +5,7 @@ package clab import ( + "context" "fmt" "os" "path/filepath" @@ -12,9 +13,13 @@ import ( "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 diff --git a/clab/test_data/topo11-ext-cont.yaml b/clab/test_data/topo11-ext-cont.yaml new file mode 100644 index 000000000..09f0276b9 --- /dev/null +++ b/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 + + diff --git a/go.mod b/go.mod index f9e90ab4d..0ee479365 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 9926b0b6c..d54a8f76f 100644 --- a/go.sum +++ b/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= diff --git a/nodes/ext_container/ext_container.go b/nodes/ext_container/ext_container.go index 7ee226794..5ff180fa7 100644 --- a/nodes/ext_container/ext_container.go +++ b/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 } diff --git a/types/types.go b/types/types.go index 81a67864e..e05cd9b59 100644 --- a/types/types.go +++ b/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 {