diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..faffd705 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,31 @@ +# Terway Project Development Guide + +## Dev environment tips + +- Use `make help` to see all available make targets and their descriptions +- Run `make generate` to generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations +- Use `make manifests` to generate CustomResourceDefinition objects +- Run `make fmt` to format Go code with `go fmt` +- Use `make vet` to run `go vet` against code with build tags + +## Testing instructions + +- Find the CI plan in the `.github/workflows` folder (check.yml, build.yml) +- Run `make quick-test` to run all tests +- Run `make lint` to run golangci-lint with the configuration in `.golangci.yml` +- Run `make lint-fix` to run golangci-lint and perform automatic fixes +- Fix any test or lint errors until the whole suite passes +- After moving files or changing imports, run `make fmt` and `make vet` to ensure code quality +- Add or update tests for the code you change, even if nobody asked + +## PR instructions + +- Title format: `[terway] ` or `[component] <Title>` +- Always run `make lint` and `make test` before committing +- Ensure `go mod tidy` and `go mod vendor` are run before submitting (checked in CI) +- Make sure all build tags are properly set (privileged, default_build) +- Verify that your changes pass the GitHub Actions workflows: + - Code formatting and linting (golangci-lint) + - Unit tests with coverage + - Module vendoring checks + - Super-linter for markdown and bash files diff --git a/Makefile b/Makefile index 61a7362d..c4f86782 100644 --- a/Makefile +++ b/Makefile @@ -53,6 +53,10 @@ vet: ## Run go vet against code. test: manifests generate fmt vet envtest datapath-test## Run tests. KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test -gcflags=all=-l -race --tags "$(GO_BUILD_TAGS)" $$(go list ./... | grep -Ev '/e2e|/mocks|/generated|/apis|/examples|/tests|/rpc|/windows|/internal/testutil') -coverprofile coverage.txt +.PHONY: test-quick +test-quick: manifests generate fmt vet envtest ## Run tests. + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test -gcflags=all=-l -race --tags "$(GO_BUILD_TAGS)" $$(go list ./... | grep -Ev '/e2e|/mocks|/generated|/apis|/examples|/tests|/rpc|/windows|/internal/testutil') -coverprofile coverage.txt + .PHONY: lint lint: golangci-lint ## Run golangci-lint linter & yamllint $(GOLANGCI_LINT) run diff --git a/cmd/terway-cli/cmd_test.go b/cmd/terway-cli/cmd_test.go index 8262f5bf..21ae37a8 100644 --- a/cmd/terway-cli/cmd_test.go +++ b/cmd/terway-cli/cmd_test.go @@ -6,8 +6,8 @@ import ( "testing" "github.com/agiledragon/gomonkey/v2" - "github.com/stretchr/testify/assert" "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" "github.com/AliyunContainerService/terway/pkg/aliyun/metadata" ) @@ -42,12 +42,12 @@ func TestPrintKV(t *testing.T) { func TestArgumentValidation(t *testing.T) { t.Run("runList argument validation", func(t *testing.T) { cmd := &cobra.Command{} - + // Test with exactly 2 arguments (should fail) err := runList(cmd, []string{"arg1", "arg2"}) assert.Error(t, err) assert.Contains(t, err.Error(), "too many arguments") - + // Test with more than 2 arguments (should fail) err = runList(cmd, []string{"arg1", "arg2", "arg3"}) assert.Error(t, err) @@ -56,17 +56,17 @@ func TestArgumentValidation(t *testing.T) { t.Run("runShow argument validation", func(t *testing.T) { cmd := &cobra.Command{} - + // Test with no arguments err := runShow(cmd, []string{}) assert.Error(t, err) assert.Contains(t, err.Error(), "no arguments") - + // Test with 2 or more arguments err = runShow(cmd, []string{"type", "name"}) assert.Error(t, err) assert.Contains(t, err.Error(), "too many arguments") - + // Test with 3 arguments err = runShow(cmd, []string{"type", "name", "extra"}) assert.Error(t, err) @@ -75,28 +75,28 @@ func TestArgumentValidation(t *testing.T) { t.Run("runExecute argument validation", func(t *testing.T) { cmd := &cobra.Command{} - + // Test with no arguments err := runExecute(cmd, []string{}) assert.Error(t, err) assert.Contains(t, err.Error(), "too few arguments") - + // Test with 1 argument err = runExecute(cmd, []string{"type"}) assert.Error(t, err) assert.Contains(t, err.Error(), "too few arguments") - + // Test with 2 arguments err = runExecute(cmd, []string{"type", "name"}) assert.Error(t, err) assert.Contains(t, err.Error(), "too few arguments") - + // Test with exactly 3 arguments should pass validation // We don't call runExecute here since it will access nil client // Instead, we verify the argument parsing logic args := []string{"type", "name", "command"} assert.True(t, len(args) >= 3, "Should have at least 3 arguments") - + // Verify argument assignment logic typ, name, command := args[0], args[1], args[2] remainingArgs := args[3:] @@ -556,4 +556,4 @@ func TestRunMetadata_ErrorCases(t *testing.T) { err := runMetadata(cmd, []string{}) assert.NoError(t, err) }) -} \ No newline at end of file +} diff --git a/daemon/builder.go b/daemon/builder.go index 8996e61d..87269a38 100644 --- a/daemon/builder.go +++ b/daemon/builder.go @@ -465,7 +465,7 @@ func (b *NetworkServiceBuilder) PostInitForCRDV2() *NetworkServiceBuilder { if b.err != nil { return b } - crdv2 := eni.NewCRDV2(b.service.k8s.NodeName(), b.namespace) + crdv2 := eni.NewCRDV2(b.service.k8s.GetRestConfig(), b.service.k8s.NodeName(), b.namespace) mgr := eni.NewManager(nil, 0, []eni.NetworkInterface{crdv2}, daemon.EniSelectionPolicy(b.config.EniSelectionPolicy), nil) svc := b.RunENIMgr(b.ctx, mgr) diff --git a/daemon/daemon.go b/daemon/daemon.go index 9236517a..dfec2508 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -86,8 +86,8 @@ var serviceLog = logf.Log.WithName("server") var _ rpc.TerwayBackendServer = (*networkService)(nil) // return resource relation in db, or return nil. -func (n *networkService) getPodResource(info *daemon.PodInfo) (daemon.PodResources, error) { - obj, err := n.resourceDB.Get(utils.PodInfoKey(info.Namespace, info.Name)) +func (n *networkService) getPodResource(podID string) (daemon.PodResources, error) { + obj, err := n.resourceDB.Get(podID) if err == nil { return obj.(daemon.PodResources), nil } @@ -98,9 +98,8 @@ func (n *networkService) getPodResource(info *daemon.PodInfo) (daemon.PodResourc return daemon.PodResources{}, err } -func (n *networkService) deletePodResource(info *daemon.PodInfo) error { - key := utils.PodInfoKey(info.Namespace, info.Name) - return n.resourceDB.Delete(key) +func (n *networkService) deletePodResource(podID string) error { + return n.resourceDB.Delete(podID) } func (n *networkService) AllocIP(ctx context.Context, r *rpc.AllocIPRequest) (*rpc.AllocIPReply, error) { @@ -163,7 +162,7 @@ func (n *networkService) AllocIP(ctx context.Context, r *rpc.AllocIPRequest) (*r } // 2. Find old resource info - oldRes, err := n.getPodResource(pod) + oldRes, err := n.getPodResource(podID) if err != nil { return nil, &types.Error{ Code: types.ErrInternalError, @@ -331,30 +330,31 @@ func (n *networkService) ReleaseIP(ctx context.Context, r *rpc.ReleaseIPRequest) // 0. Get pod Info pod, err := n.k8s.GetPod(ctx, r.K8SPodNamespace, r.K8SPodName, true) if err != nil { - if k8sErr.IsNotFound(err) { - return reply, nil + if !k8sErr.IsNotFound(err) { + l.Error(err, "get pod failed") + // ignore error, do not block delete } - return nil, err } cni := &daemon.CNI{ PodName: r.K8SPodName, PodNamespace: r.K8SPodNamespace, PodID: podID, - PodUID: pod.PodUID, + } + + var podIpStickTime time.Duration + if pod != nil { + cni.PodUID = pod.PodUID + podIpStickTime = pod.IPStickTime } // 1. Init Context - oldRes, err := n.getPodResource(pod) + oldRes, err := n.getPodResource(podID) if err != nil { return nil, err } - if !n.verifyPodNetworkType(pod.PodNetworkType) { - return nil, fmt.Errorf("unexpect pod network type allocate, maybe daemon mode changed: %+v", pod.PodNetworkType) - } - if oldRes.ContainerID != nil { if r.K8SPodInfraContainerId != *oldRes.ContainerID { l.Info("cni request not match stored resource, ignored", "old", *oldRes.ContainerID) @@ -365,7 +365,7 @@ func (n *networkService) ReleaseIP(ctx context.Context, r *rpc.ReleaseIPRequest) cni.PodUID = oldRes.PodInfo.PodUID } - if n.ipamType == types.IPAMTypeCRD || pod.IPStickTime == 0 { + if n.ipamType == types.IPAMTypeCRD || podIpStickTime <= 0 { for _, resource := range oldRes.Resources { res := parseNetworkResource(resource) if res == nil { @@ -379,7 +379,7 @@ func (n *networkService) ReleaseIP(ctx context.Context, r *rpc.ReleaseIPRequest) return nil, err } } - err = n.deletePodResource(pod) + err = n.deletePodResource(podID) if err != nil { return nil, fmt.Errorf("error delete pod resource: %w", err) } @@ -388,6 +388,7 @@ func (n *networkService) ReleaseIP(ctx context.Context, r *rpc.ReleaseIPRequest) return reply, nil } +// GetIPInfo return cached alloc ip info func (n *networkService) GetIPInfo(ctx context.Context, r *rpc.GetInfoRequest) (*rpc.GetInfoReply, error) { podID := utils.PodInfoKey(r.K8SPodNamespace, r.K8SPodName) log := logf.FromContext(ctx) @@ -409,16 +410,6 @@ func (n *networkService) GetIPInfo(ctx context.Context, r *rpc.GetInfoRequest) ( var err error - // 0. Get pod Info - pod, err := n.k8s.GetPod(ctx, r.K8SPodNamespace, r.K8SPodName, true) - if err != nil { - return nil, &types.Error{ - Code: types.ErrInvalidArgsErrCode, - Msg: err.Error(), - R: err, - } - } - // 1. Init Context reply := &rpc.GetInfoReply{ Success: true, @@ -426,10 +417,10 @@ func (n *networkService) GetIPInfo(ctx context.Context, r *rpc.GetInfoRequest) ( IPv6: n.enableIPv6, } - switch pod.PodNetworkType { - case daemon.PodNetworkTypeENIMultiIP: + switch n.daemonMode { + case daemon.ModeENIMultiIP: reply.IPType = rpc.IPType_TypeENIMultiIP - case daemon.PodNetworkTypeVPCENI: + case daemon.ModeENIOnly: reply.IPType = rpc.IPType_TypeVPCENI default: return nil, &types.Error{ @@ -439,7 +430,7 @@ func (n *networkService) GetIPInfo(ctx context.Context, r *rpc.GetInfoRequest) ( } // 2. Find old resource info - oldRes, err := n.getPodResource(pod) + oldRes, err := n.getPodResource(podID) if err != nil { return nil, &types.Error{ Code: types.ErrInternalError, @@ -448,12 +439,6 @@ func (n *networkService) GetIPInfo(ctx context.Context, r *rpc.GetInfoRequest) ( } } - if !n.verifyPodNetworkType(pod.PodNetworkType) { - return nil, &types.Error{ - Code: types.ErrInvalidArgsErrCode, - Msg: "Unexpected network type, maybe daemon mode changed", - } - } if oldRes.ContainerID != nil { if r.K8SPodInfraContainerId != *oldRes.ContainerID { log.Info("cni request not match stored resource, ignored", "old", *oldRes.ContainerID) @@ -640,7 +625,7 @@ func (n *networkService) gcPods(ctx context.Context) error { } } - err = n.deletePodResource(podRes.PodInfo) + err = n.deletePodResource(podID) if err != nil { return err } diff --git a/daemon/server_test.go b/daemon/server_test.go index 07f56f85..4168d6d9 100644 --- a/daemon/server_test.go +++ b/daemon/server_test.go @@ -508,17 +508,22 @@ func TestReleaseIP(t *testing.T) { }, }, { - name: "error when pod not found but not NotFound error", + name: "non-NotFound error when getting pod - should continue release", request: &rpc.ReleaseIPRequest{ K8SPodNamespace: "default", K8SPodName: "test-pod", K8SPodInfraContainerId: "container-123", }, - expectedError: true, + expectedError: false, // ReleaseIP ignores non-NotFound errors to not block delete operations setupMocks: func(patches *gomonkey.Patches, ns *networkService) { // Mock k8s.GetPod to return non-NotFound error mockK8s := ns.k8s.(*mocks.Kubernetes) mockK8s.On("GetPod", mock.Anything, "default", "test-pod", true).Return((*daemon.PodInfo)(nil), assert.AnError) + + // Mock resourceDB.Get to return ErrNotFound (no existing resource) + patches.ApplyMethodFunc(ns.resourceDB, "Get", func(key string) (interface{}, error) { + return nil, storage.ErrNotFound + }) }, }, { @@ -826,14 +831,7 @@ func TestGetIPInfo(t *testing.T) { expectedError: false, expectedIPType: rpc.IPType_TypeENIMultiIP, setupMocks: func(patches *gomonkey.Patches, ns *networkService) { - // Mock k8s.GetPod - mockK8s := ns.k8s.(*mocks.Kubernetes) - mockK8s.On("GetPod", mock.Anything, "default", "test-pod", true).Return(&daemon.PodInfo{ - Namespace: "default", - Name: "test-pod", - PodUID: "pod-uid-123", - PodNetworkType: daemon.PodNetworkTypeENIMultiIP, - }, nil) + // GetIPInfo does NOT call GetPod - it only uses resourceDB // Mock resourceDB.Get to return existing resource patches.ApplyMethodFunc(ns.resourceDB, "Get", func(key string) (interface{}, error) { @@ -877,14 +875,7 @@ func TestGetIPInfo(t *testing.T) { // Set daemon mode to ENIOnly for VPCENI ns.daemonMode = daemon.ModeENIOnly - // Mock k8s.GetPod - mockK8s := ns.k8s.(*mocks.Kubernetes) - mockK8s.On("GetPod", mock.Anything, "default", "test-pod", true).Return(&daemon.PodInfo{ - Namespace: "default", - Name: "test-pod", - PodUID: "pod-uid-123", - PodNetworkType: daemon.PodNetworkTypeVPCENI, - }, nil) + // GetIPInfo does NOT call GetPod - it only uses resourceDB // Mock resourceDB.Get to return existing resource patches.ApplyMethodFunc(ns.resourceDB, "Get", func(key string) (interface{}, error) { @@ -901,17 +892,21 @@ func TestGetIPInfo(t *testing.T) { }, }, { - name: "error when pod not found", + name: "success when resource not in DB - return empty netconf", request: &rpc.GetInfoRequest{ K8SPodNamespace: "default", K8SPodName: "test-pod", K8SPodInfraContainerId: "container-123", }, - expectedError: true, + expectedError: false, // GetIPInfo returns success with empty netconf when resource not found + expectedIPType: rpc.IPType_TypeENIMultiIP, setupMocks: func(patches *gomonkey.Patches, ns *networkService) { - // Mock k8s.GetPod to return error - mockK8s := ns.k8s.(*mocks.Kubernetes) - mockK8s.On("GetPod", mock.Anything, "default", "test-pod", true).Return((*daemon.PodInfo)(nil), assert.AnError) + // GetIPInfo does NOT call GetPod + + // Mock resourceDB.Get to return ErrNotFound + patches.ApplyMethodFunc(ns.resourceDB, "Get", func(key string) (interface{}, error) { + return nil, storage.ErrNotFound + }) }, }, { @@ -923,23 +918,16 @@ func TestGetIPInfo(t *testing.T) { }, expectedError: true, setupMocks: func(patches *gomonkey.Patches, ns *networkService) { - // Mock k8s.GetPod - mockK8s := ns.k8s.(*mocks.Kubernetes) - mockK8s.On("GetPod", mock.Anything, "default", "test-pod", true).Return(&daemon.PodInfo{ - Namespace: "default", - Name: "test-pod", - PodUID: "pod-uid-123", - PodNetworkType: daemon.PodNetworkTypeENIMultiIP, - }, nil) + // GetIPInfo does NOT call GetPod - // Mock resourceDB.Get to return error + // Mock resourceDB.Get to return error (not ErrNotFound) patches.ApplyMethodFunc(ns.resourceDB, "Get", func(key string) (interface{}, error) { return nil, assert.AnError }) }, }, { - name: "error when pod network type is unknown", + name: "error when daemon mode is unknown", request: &rpc.GetInfoRequest{ K8SPodNamespace: "default", K8SPodName: "test-pod", @@ -947,36 +935,26 @@ func TestGetIPInfo(t *testing.T) { }, expectedError: true, setupMocks: func(patches *gomonkey.Patches, ns *networkService) { - // Mock k8s.GetPod with unknown network type - mockK8s := ns.k8s.(*mocks.Kubernetes) - mockK8s.On("GetPod", mock.Anything, "default", "test-pod", true).Return(&daemon.PodInfo{ - Namespace: "default", - Name: "test-pod", - PodUID: "pod-uid-123", - PodNetworkType: "UnknownType", // Unknown network type - }, nil) + // Set an invalid daemon mode + ns.daemonMode = "UnknownMode" + + // GetIPInfo does NOT call GetPod }, }, { - name: "error when daemon mode doesn't match pod network type", + name: "success with ENIOnly daemon mode", request: &rpc.GetInfoRequest{ K8SPodNamespace: "default", K8SPodName: "test-pod", K8SPodInfraContainerId: "container-123", }, - expectedError: true, + expectedError: false, + expectedIPType: rpc.IPType_TypeVPCENI, setupMocks: func(patches *gomonkey.Patches, ns *networkService) { - // Set daemon mode to ENIOnly but pod is ENIMultiIP + // Set daemon mode to ENIOnly ns.daemonMode = daemon.ModeENIOnly - // Mock k8s.GetPod - mockK8s := ns.k8s.(*mocks.Kubernetes) - mockK8s.On("GetPod", mock.Anything, "default", "test-pod", true).Return(&daemon.PodInfo{ - Namespace: "default", - Name: "test-pod", - PodUID: "pod-uid-123", - PodNetworkType: daemon.PodNetworkTypeENIMultiIP, // Mismatch with daemon mode - }, nil) + // GetIPInfo does NOT call GetPod // Mock resourceDB.Get to return existing resource patches.ApplyMethodFunc(ns.resourceDB, "Get", func(key string) (interface{}, error) { @@ -1015,14 +993,7 @@ func TestGetIPInfo(t *testing.T) { expectedError: false, expectedIPType: rpc.IPType_TypeENIMultiIP, setupMocks: func(patches *gomonkey.Patches, ns *networkService) { - // Mock k8s.GetPod - mockK8s := ns.k8s.(*mocks.Kubernetes) - mockK8s.On("GetPod", mock.Anything, "default", "test-pod", true).Return(&daemon.PodInfo{ - Namespace: "default", - Name: "test-pod", - PodUID: "pod-uid-123", - PodNetworkType: daemon.PodNetworkTypeENIMultiIP, - }, nil) + // GetIPInfo does NOT call GetPod // Mock resourceDB.Get with different container ID patches.ApplyMethodFunc(ns.resourceDB, "Get", func(key string) (interface{}, error) { @@ -1048,14 +1019,7 @@ func TestGetIPInfo(t *testing.T) { expectedError: false, expectedIPType: rpc.IPType_TypeENIMultiIP, setupMocks: func(patches *gomonkey.Patches, ns *networkService) { - // Mock k8s.GetPod - mockK8s := ns.k8s.(*mocks.Kubernetes) - mockK8s.On("GetPod", mock.Anything, "default", "test-pod", true).Return(&daemon.PodInfo{ - Namespace: "default", - Name: "test-pod", - PodUID: "pod-uid-123", - PodNetworkType: daemon.PodNetworkTypeENIMultiIP, - }, nil) + // GetIPInfo does NOT call GetPod // Mock resourceDB.Get with invalid JSON patches.ApplyMethodFunc(ns.resourceDB, "Get", func(key string) (interface{}, error) { diff --git a/pkg/aliyun/client/hdeni_test.go b/pkg/aliyun/client/hdeni_test.go index fead16ce..7d550f11 100644 --- a/pkg/aliyun/client/hdeni_test.go +++ b/pkg/aliyun/client/hdeni_test.go @@ -58,4 +58,4 @@ func TestEFLOService_DetachHDENI(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "unsupported") -} \ No newline at end of file +} diff --git a/pkg/controller/multi-ip/node/eni_test.go b/pkg/controller/multi-ip/node/eni_test.go index 75c2e467..76849e2f 100644 --- a/pkg/controller/multi-ip/node/eni_test.go +++ b/pkg/controller/multi-ip/node/eni_test.go @@ -274,7 +274,7 @@ func Test_mergeIPMap(t *testing.T) { { name: "primary case", args: args{ - log: logr.Discard(), + log: logr.Discard(), remote: map[string]*networkv1beta1.IP{ // somehow primary ip is not in remote }, diff --git a/pkg/eni/crdv2.go b/pkg/eni/crdv2.go index 3c51274a..f4403ccd 100644 --- a/pkg/eni/crdv2.go +++ b/pkg/eni/crdv2.go @@ -63,12 +63,7 @@ type CRDV2 struct { podENINotifier *Notifier } -func NewCRDV2(nodeName, namespace string) *CRDV2 { - restConfig := ctrl.GetConfigOrDie() - return newCRDV2(restConfig, nodeName, namespace) -} - -func newCRDV2(restConfig *rest.Config, nodeName, namespace string) *CRDV2 { +func NewCRDV2(restConfig *rest.Config, nodeName, namespace string) *CRDV2 { options := ctrl.Options{ Scheme: types.Scheme, HealthProbeBindAddress: "0", @@ -110,11 +105,18 @@ func newCRDV2(restConfig *rest.Config, nodeName, namespace string) *CRDV2 { Transform: nil, }, &corev1.Pod{}: { - Field: client.MatchingFieldsSelector{ - Selector: fields.SelectorFromSet(fields.Set{"spec.nodeName": nodeName}), - }, + Field: fields.AndSelectors( + fields.OneTermEqualSelector("spec.nodeName", nodeName), + fields.OneTermNotEqualSelector("status.phase", string(corev1.PodSucceeded)), + fields.OneTermNotEqualSelector("status.phase", string(corev1.PodFailed)), + ), Transform: func(i interface{}) (interface{}, error) { if pod, ok := i.(*corev1.Pod); ok { + if pod.Spec.HostNetwork { + pod.Annotations = nil + pod.Labels = nil + } + pod.Spec.Volumes = nil pod.Spec.EphemeralContainers = nil pod.Spec.SecurityContext = nil diff --git a/pkg/eni/crdv2_new_test.go b/pkg/eni/crdv2_new_test.go index 0197032f..aadfc260 100644 --- a/pkg/eni/crdv2_new_test.go +++ b/pkg/eni/crdv2_new_test.go @@ -18,7 +18,7 @@ var _ = Describe("start controller", func() { ) It("New Controller", func() { - ctrl := newCRDV2(testEnv.Config, nodeName, "default") + ctrl := NewCRDV2(testEnv.Config, nodeName, "default") Expect(ctrl).NotTo(BeNil()) ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) diff --git a/pkg/eni/manager_test.go b/pkg/eni/manager_test.go index 9c831427..8ca813a7 100644 --- a/pkg/eni/manager_test.go +++ b/pkg/eni/manager_test.go @@ -9,9 +9,8 @@ import ( "time" "github.com/stretchr/testify/assert" - corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/AliyunContainerService/terway/pkg/k8s/mocks" "github.com/AliyunContainerService/terway/types" "github.com/AliyunContainerService/terway/types/daemon" ) @@ -84,7 +83,7 @@ func (s *success) Run(ctx context.Context, podResources []daemon.PodResources, w func TestManagerAllocateReturnsResourcesWhenSuccessful(t *testing.T) { mockNI := &success{} - manager := NewManager(nil, 0, []NetworkInterface{mockNI}, daemon.EniSelectionPolicyMostIPs, &FakeK8s{}) + manager := NewManager(nil, 0, []NetworkInterface{mockNI}, daemon.EniSelectionPolicyMostIPs, mocks.NewKubernetes(t)) request := NewLocalIPRequest() resources, err := manager.Allocate(context.Background(), &daemon.CNI{}, &AllocRequest{ @@ -108,7 +107,7 @@ func TestManagerAllocateSelectionPolicy(t *testing.T) { } { - manager := NewManager(nil, 0, []NetworkInterface{mockNI, mockNI2}, daemon.EniSelectionPolicyMostIPs, &FakeK8s{}) + manager := NewManager(nil, 0, []NetworkInterface{mockNI, mockNI2}, daemon.EniSelectionPolicyMostIPs, mocks.NewKubernetes(t)) request := NewLocalIPRequest() resources, err := manager.Allocate(context.Background(), &daemon.CNI{}, &AllocRequest{ @@ -121,7 +120,7 @@ func TestManagerAllocateSelectionPolicy(t *testing.T) { } { - manager := NewManager(nil, 0, []NetworkInterface{mockNI, mockNI2}, daemon.EniSelectionPolicyLeastIPs, &FakeK8s{}) + manager := NewManager(nil, 0, []NetworkInterface{mockNI, mockNI2}, daemon.EniSelectionPolicyLeastIPs, mocks.NewKubernetes(t)) request := NewLocalIPRequest() resources, err := manager.Allocate(context.Background(), &daemon.CNI{}, &AllocRequest{ @@ -135,7 +134,7 @@ func TestManagerAllocateSelectionPolicy(t *testing.T) { } func TestManagerAllocateReturnsErrorWhenNoBackendCanHandleAllocation(t *testing.T) { - manager := NewManager(nil, 0, []NetworkInterface{}, daemon.EniSelectionPolicyMostIPs, &FakeK8s{}) + manager := NewManager(nil, 0, []NetworkInterface{}, daemon.EniSelectionPolicyMostIPs, mocks.NewKubernetes(t)) request := NewLocalIPRequest() _, err := manager.Allocate(context.Background(), &daemon.CNI{}, &AllocRequest{ @@ -147,7 +146,7 @@ func TestManagerAllocateReturnsErrorWhenNoBackendCanHandleAllocation(t *testing. func TestManagerAllocateWithTimeoutWhenAllocationFails(t *testing.T) { mockNI := &timeOut{} - manager := NewManager(nil, 0, []NetworkInterface{mockNI}, daemon.EniSelectionPolicyMostIPs, &FakeK8s{}) + manager := NewManager(nil, 0, []NetworkInterface{mockNI}, daemon.EniSelectionPolicyMostIPs, mocks.NewKubernetes(t)) ctx, cancel := context.WithTimeout(context.Background(), time.Second) defer cancel() @@ -159,91 +158,6 @@ func TestManagerAllocateWithTimeoutWhenAllocationFails(t *testing.T) { assert.NotNil(t, err) } -type FakeK8s struct{} - -func (f *FakeK8s) NodeName() string { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) GetLocalPods() ([]*daemon.PodInfo, error) { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) GetPod(ctx context.Context, namespace, name string, cache bool) (*daemon.PodInfo, error) { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) GetServiceCIDR() *types.IPNetSet { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) SetNodeAllocatablePod(count int) error { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) PatchNodeAnnotations(anno map[string]string) error { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) PatchPodIPInfo(info *daemon.PodInfo, ips string) error { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) PatchNodeIPResCondition(status corev1.ConditionStatus, reason, message string) error { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) RecordNodeEvent(eventType, reason, message string) { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) RecordPodEvent(podName, podNamespace, eventType, reason, message string) error { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) GetNodeDynamicConfigLabel() string { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) GetDynamicConfigWithName(ctx context.Context, name string) (string, error) { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) SetCustomStatefulWorkloadKinds(kinds []string) error { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) GetTrunkID() string { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) GetClient() client.Client { - //TODO implement me - panic("implement me") -} - -func (f *FakeK8s) PodExist(namespace, name string) (bool, error) { - panic("implement me") -} - -func (f *FakeK8s) Node() *corev1.Node { - panic("implement me") -} - func TestManager_calculateToDel(t *testing.T) { tests := []struct { name string diff --git a/pkg/k8s/k8s.go b/pkg/k8s/k8s.go index 0b6524ba..fdce40b4 100644 --- a/pkg/k8s/k8s.go +++ b/pkg/k8s/k8s.go @@ -25,6 +25,7 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" typedv1 "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" @@ -101,6 +102,8 @@ type Kubernetes interface { NodeName() string Node() *corev1.Node + + GetRestConfig() *rest.Config } // NewK8S return Kubernetes service by pod spec and daemon mode @@ -162,6 +165,7 @@ func NewK8S(daemonMode string, globalConfig *daemon.Config, namespace string) (K recorder: recorder, Locker: &sync.RWMutex{}, enableErdma: globalConfig.EnableERDMA, + restConfig: restConfig, } svcCIDR := &types.IPNetSet{} @@ -201,6 +205,8 @@ type k8s struct { statefulWorkloadKindSet sets.Set[string] enableErdma bool + restConfig *rest.Config + sync.Locker } @@ -350,10 +356,19 @@ func (k *k8s) GetPod(ctx context.Context, namespace, name string, cache bool) (* item := &storageItem{ Pod: podInfo, } - err = k.storage.Put(key, item) - if err != nil { - return nil, err + + if !k.shouldHandlePod(pod) { + return nil, apierrors.NewNotFound(corev1.Resource("pod"), name) + } + + // nb(l1b0k): for backward compatibility + if podInfo.IPStickTime > 0 { + err = k.storage.Put(key, item) + if err != nil { + return nil, err + } } + return podInfo, nil } @@ -365,7 +380,7 @@ func (k *k8s) PodExist(namespace, name string) (bool, error) { } return false, err } - if pod.Spec.NodeName != k.nodeName { + if !k.shouldHandlePod(pod) { return false, nil } @@ -374,7 +389,11 @@ func (k *k8s) PodExist(namespace, name string) (bool, error) { func (k *k8s) GetLocalPods() ([]*daemon.PodInfo, error) { options := metav1.ListOptions{ - FieldSelector: fields.OneTermEqualSelector("spec.nodeName", k.nodeName).String(), + FieldSelector: fields.AndSelectors( + fields.OneTermEqualSelector("spec.nodeName", k.nodeName), + fields.OneTermNotEqualSelector("status.phase", string(corev1.PodSucceeded)), + fields.OneTermNotEqualSelector("status.phase", string(corev1.PodFailed)), + ).String(), ResourceVersion: "0", } podList := &corev1.PodList{} @@ -385,7 +404,7 @@ func (k *k8s) GetLocalPods() ([]*daemon.PodInfo, error) { } var ret []*daemon.PodInfo for _, pod := range podList.Items { - if types.IgnoredByTerway(pod.Labels) { + if !k.shouldHandlePod(&pod) { continue } @@ -519,7 +538,15 @@ func (k *k8s) Node() *corev1.Node { return k.node } -func podNetworkType(daemonMode string, pod *corev1.Pod) string { +func (k *k8s) GetRestConfig() *rest.Config { + return k.restConfig +} + +func (k *k8s) shouldHandlePod(pod *corev1.Pod) bool { + return pod.Spec.NodeName == k.nodeName && !types.IgnoredByTerway(pod.Labels) && !pod.Spec.HostNetwork +} + +func podNetworkType(daemonMode string) string { switch daemonMode { case daemon.ModeENIMultiIP: return daemon.PodNetworkTypeENIMultiIP @@ -539,7 +566,7 @@ func convertPod(daemonMode string, enableErdma bool, statefulWorkloadKindSet set CreateTime: pod.CreationTimestamp.Time, } - pi.PodNetworkType = podNetworkType(daemonMode, pod) + pi.PodNetworkType = podNetworkType(daemonMode) for _, str := range pod.Status.PodIPs { pi.PodIPs.SetIP(str.IP) diff --git a/pkg/k8s/k8s_test.go b/pkg/k8s/k8s_test.go index eda86880..f27e6964 100644 --- a/pkg/k8s/k8s_test.go +++ b/pkg/k8s/k8s_test.go @@ -13,6 +13,7 @@ import ( "github.com/AliyunContainerService/terway/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" @@ -28,57 +29,116 @@ import ( ) func TestK8s_PodExist(t *testing.T) { - // Setup - scheme := runtime.NewScheme() - _ = corev1.AddToScheme(scheme) + // Test existing pod on same node - should return true + t.Run("pod on same node should exist", func(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + Spec: corev1.PodSpec{ + NodeName: "test-node", + }, + } - pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod", - Namespace: "default", - }, - Spec: corev1.PodSpec{ - NodeName: "test-node", - }, - } + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(pod).Build() - fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(pod).Build() + k8sObj := &k8s{ + client: fakeClient, + nodeName: "test-node", + } - k8sObj := &k8s{ - client: fakeClient, - nodeName: "test-node", - } + exist, err := k8sObj.PodExist("default", "test-pod") + assert.NoError(t, err) + assert.True(t, exist, "Pod on same node should exist") + }) - // Test existing pod on node - exist, err := k8sObj.PodExist("default", "test-pod") - assert.NoError(t, err) - assert.True(t, exist) + // Test non-existent pod - should return false + t.Run("non-existent pod should not exist", func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - // Test non-existent pod - exist, err = k8sObj.PodExist("default", "non-existent") - assert.NoError(t, err) - assert.False(t, exist) + k8sObj := &k8s{ + client: fakeClient, + nodeName: "test-node", + } - // Test pod on different node - podDifferentNode := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "other-pod", - Namespace: "default", - }, - Spec: corev1.PodSpec{ - NodeName: "other-node", - }, - } + exist, err := k8sObj.PodExist("default", "non-existent") + assert.NoError(t, err) + assert.False(t, exist, "Non-existent pod should not exist") + }) - fakeClient2 := fake.NewClientBuilder().WithScheme(scheme).WithObjects(podDifferentNode).Build() - k8sObj2 := &k8s{ - client: fakeClient2, - nodeName: "test-node", - } + // Test pod on different node - should return false (shouldHandlePod returns false) + t.Run("pod on different node should not exist", func(t *testing.T) { + podDifferentNode := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-pod", + Namespace: "default", + }, + Spec: corev1.PodSpec{ + NodeName: "other-node", + }, + } - exist, err = k8sObj2.PodExist("default", "other-pod") - assert.NoError(t, err) - assert.False(t, exist) + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(podDifferentNode).Build() + k8sObj := &k8s{ + client: fakeClient, + nodeName: "test-node", + } + + exist, err := k8sObj.PodExist("default", "other-pod") + assert.NoError(t, err) + assert.False(t, exist, "Pod on different node should not exist for this k8s instance") + }) + + // Test pod with host network - should return false (shouldHandlePod returns false) + t.Run("pod with host network should not exist", func(t *testing.T) { + podHostNetwork := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "host-network-pod", + Namespace: "default", + }, + Spec: corev1.PodSpec{ + NodeName: "test-node", + HostNetwork: true, + }, + } + + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(podHostNetwork).Build() + k8sObj := &k8s{ + client: fakeClient, + nodeName: "test-node", + } + + exist, err := k8sObj.PodExist("default", "host-network-pod") + assert.NoError(t, err) + assert.False(t, exist, "Pod with host network should not exist for terway") + }) + + // Test pod with ignore label - should return false (shouldHandlePod returns false) + t.Run("pod with ignore label should not exist", func(t *testing.T) { + podIgnored := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ignored-pod", + Namespace: "default", + Labels: map[string]string{ + types.IgnoreByTerway: "true", + }, + }, + Spec: corev1.PodSpec{ + NodeName: "test-node", + }, + } + + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(podIgnored).Build() + k8sObj := &k8s{ + client: fakeClient, + nodeName: "test-node", + } + + exist, err := k8sObj.PodExist("default", "ignored-pod") + assert.NoError(t, err) + assert.False(t, exist, "Pod with ignore label should not exist for terway") + }) } func TestK8s_GetLocalPods_ENIOnlyMode(t *testing.T) { @@ -806,12 +866,10 @@ func TestIsERDMA(t *testing.T) { } func TestPodNetworkType(t *testing.T) { - pod := &corev1.Pod{} - - result := podNetworkType(daemon.ModeENIMultiIP, pod) + result := podNetworkType(daemon.ModeENIMultiIP) assert.Equal(t, daemon.PodNetworkTypeENIMultiIP, result) - result = podNetworkType(daemon.ModeENIOnly, pod) + result = podNetworkType(daemon.ModeENIOnly) assert.Equal(t, daemon.PodNetworkTypeVPCENI, result) } @@ -1382,163 +1440,255 @@ func TestSetSvcCIDR_APIServerError(t *testing.T) { // ============================================================================== func TestGetPod(t *testing.T) { - tests := []struct { - name string - podExists bool - useCache bool - storageExists bool - storageError error - expectError bool - description string - }{ - { - name: "get existing pod with cache", - podExists: true, - useCache: true, - storageExists: false, - expectError: false, - description: "Should successfully get pod from Kubernetes API", - }, - { - name: "get existing pod without cache", - podExists: true, - useCache: false, - storageExists: false, - expectError: false, - description: "Should successfully get pod from Kubernetes API without cache", - }, - { - name: "get non-existent pod with storage fallback", - podExists: false, - useCache: true, - storageExists: true, - expectError: false, - description: "Should get pod from storage when not found in Kubernetes API", - }, - { - name: "get non-existent pod without storage fallback", - podExists: false, - useCache: true, - storageExists: false, - expectError: true, - description: "Should return error when pod not found in API and storage", - }, - { - name: "storage error on fallback", - podExists: false, - useCache: true, - storageExists: false, - storageError: fmt.Errorf("storage error"), - expectError: true, - description: "Should return storage error when storage operation fails", - }, - } + // Test: get existing pod with cache - pod on same node should be returned + t.Run("get existing pod with cache", func(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + Spec: corev1.PodSpec{ + NodeName: "test-node", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + PodIP: "10.0.0.1", + }, + } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Setup fake client - builder := fake.NewClientBuilder().WithScheme(scheme.Scheme) - if tt.podExists { - pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod", - Namespace: "default", - }, - Spec: corev1.PodSpec{ - NodeName: "test-node", - }, - Status: corev1.PodStatus{ - Phase: corev1.PodRunning, - PodIP: "10.0.0.1", - }, - } - builder.WithObjects(pod) - } - client := builder.Build() + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(pod).Build() - // Setup mock storage - mockStore := &mockStorage{data: make(map[string]interface{})} + // Setup mock storage (should not be accessed for existing pod) + mockStore := &mockStorage{data: make(map[string]interface{})} - // Only set up storage expectations when pod doesn't exist in Kubernetes - if !tt.podExists { - if tt.storageExists { - storageItem := &storageItem{ - Pod: &daemon.PodInfo{ - Name: "test-pod", - Namespace: "default", - }, - } - mockStore.data["default/test-pod"] = storageItem - mockStore.On("Get", "default/test-pod").Return(storageItem, nil) - } else if tt.storageError != nil { - mockStore.On("Get", "default/test-pod").Return(nil, tt.storageError) - } else { - mockStore.On("Get", "default/test-pod").Return(nil, storage.ErrNotFound) - } - } else { - // When pod exists in Kubernetes, we should expect Put to be called - mockStore.On("Put", "default/test-pod", mock.Anything).Return(nil) - } + k8sObj := &k8s{ + client: fakeClient, + storage: mockStore, + mode: daemon.ModeENIMultiIP, + enableErdma: false, + statefulWorkloadKindSet: sets.New[string](), + nodeName: "test-node", + } - k8sObj := &k8s{ - client: client, - storage: mockStore, - mode: daemon.ModeENIMultiIP, - enableErdma: false, - statefulWorkloadKindSet: sets.New[string](), - } + podInfo, err := k8sObj.GetPod(context.Background(), "default", "test-pod", true) - // Execute GetPod - podInfo, err := k8sObj.GetPod(context.Background(), "default", "test-pod", tt.useCache) + require.NoError(t, err, "Should successfully get pod from Kubernetes API") + require.NotNil(t, podInfo, "PodInfo should not be nil") + assert.Equal(t, "test-pod", podInfo.Name, "Pod name should match") + assert.Equal(t, "default", podInfo.Namespace, "Pod namespace should match") + }) - if tt.expectError { - assert.Error(t, err, tt.description) - assert.Nil(t, podInfo, "PodInfo should be nil on error") - } else { - assert.NoError(t, err, tt.description) - assert.NotNil(t, podInfo, "PodInfo should not be nil") - assert.Equal(t, "test-pod", podInfo.Name, "Pod name should match") - assert.Equal(t, "default", podInfo.Namespace, "Pod namespace should match") - } + // Test: get existing pod without cache + t.Run("get existing pod without cache", func(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + Spec: corev1.PodSpec{ + NodeName: "test-node", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + PodIP: "10.0.0.1", + }, + } - mockStore.AssertExpectations(t) - }) - } -} + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(pod).Build() + mockStore := &mockStorage{data: make(map[string]interface{})} -func TestGetPod_StoragePutError(t *testing.T) { - // Test case where storage put fails - pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod", - Namespace: "default", - }, - Spec: corev1.PodSpec{ - NodeName: "test-node", - }, - Status: corev1.PodStatus{ - Phase: corev1.PodRunning, - }, - } + k8sObj := &k8s{ + client: fakeClient, + storage: mockStore, + mode: daemon.ModeENIMultiIP, + enableErdma: false, + statefulWorkloadKindSet: sets.New[string](), + nodeName: "test-node", + } - client := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(pod).Build() + podInfo, err := k8sObj.GetPod(context.Background(), "default", "test-pod", false) - mockStore := &mockStorage{} - mockStore.On("Put", "default/test-pod", mock.Anything).Return(fmt.Errorf("storage put error")) + require.NoError(t, err, "Should successfully get pod from Kubernetes API without cache") + require.NotNil(t, podInfo, "PodInfo should not be nil") + assert.Equal(t, "test-pod", podInfo.Name, "Pod name should match") + assert.Equal(t, "default", podInfo.Namespace, "Pod namespace should match") + }) - k8sObj := &k8s{ - client: client, - storage: mockStore, - mode: daemon.ModeENIMultiIP, - enableErdma: false, - statefulWorkloadKindSet: sets.New[string](), - } + // Test: get non-existent pod with storage fallback + t.Run("get non-existent pod with storage fallback", func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + + // Setup mock storage with stored pod + mockStore := &mockStorage{data: make(map[string]interface{})} + storageItem := &storageItem{ + Pod: &daemon.PodInfo{ + Name: "test-pod", + Namespace: "default", + }, + } + mockStore.data["default/test-pod"] = storageItem + mockStore.On("Get", "default/test-pod").Return(storageItem, nil) + + k8sObj := &k8s{ + client: fakeClient, + storage: mockStore, + mode: daemon.ModeENIMultiIP, + enableErdma: false, + statefulWorkloadKindSet: sets.New[string](), + nodeName: "test-node", + } + + podInfo, err := k8sObj.GetPod(context.Background(), "default", "test-pod", true) + + require.NoError(t, err, "Should get pod from storage when not found in Kubernetes API") + require.NotNil(t, podInfo, "PodInfo should not be nil") + assert.Equal(t, "test-pod", podInfo.Name, "Pod name should match") + assert.Equal(t, "default", podInfo.Namespace, "Pod namespace should match") + mockStore.AssertExpectations(t) + }) + + // Test: get non-existent pod without storage fallback + t.Run("get non-existent pod without storage fallback", func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + + mockStore := &mockStorage{data: make(map[string]interface{})} + mockStore.On("Get", "default/test-pod").Return(nil, storage.ErrNotFound) + + k8sObj := &k8s{ + client: fakeClient, + storage: mockStore, + mode: daemon.ModeENIMultiIP, + enableErdma: false, + statefulWorkloadKindSet: sets.New[string](), + nodeName: "test-node", + } + + podInfo, err := k8sObj.GetPod(context.Background(), "default", "test-pod", true) + + assert.Error(t, err, "Should return error when pod not found in API and storage") + assert.Nil(t, podInfo, "PodInfo should be nil on error") + mockStore.AssertExpectations(t) + }) + + // Test: storage error on fallback + t.Run("storage error on fallback", func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + + mockStore := &mockStorage{data: make(map[string]interface{})} + mockStore.On("Get", "default/test-pod").Return(nil, fmt.Errorf("storage error")) + + k8sObj := &k8s{ + client: fakeClient, + storage: mockStore, + mode: daemon.ModeENIMultiIP, + enableErdma: false, + statefulWorkloadKindSet: sets.New[string](), + nodeName: "test-node", + } + + podInfo, err := k8sObj.GetPod(context.Background(), "default", "test-pod", true) + + assert.Error(t, err, "Should return storage error when storage operation fails") + assert.Nil(t, podInfo, "PodInfo should be nil on error") + mockStore.AssertExpectations(t) + }) + + // Test: pod on different node should return NotFound (shouldHandlePod returns false) + t.Run("pod on different node should return NotFound", func(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + Spec: corev1.PodSpec{ + NodeName: "other-node", // Different from k8sObj.nodeName + }, + } + + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(pod).Build() + mockStore := &mockStorage{data: make(map[string]interface{})} + + k8sObj := &k8s{ + client: fakeClient, + storage: mockStore, + mode: daemon.ModeENIMultiIP, + enableErdma: false, + statefulWorkloadKindSet: sets.New[string](), + nodeName: "test-node", + } + + podInfo, err := k8sObj.GetPod(context.Background(), "default", "test-pod", true) - podInfo, err := k8sObj.GetPod(context.Background(), "default", "test-pod", true) + assert.Error(t, err, "Should return error for pod on different node") + assert.True(t, apierrors.IsNotFound(err), "Error should be NotFound") + assert.Nil(t, podInfo, "PodInfo should be nil") + }) - assert.Error(t, err, "Should return error when storage put fails") - assert.Nil(t, podInfo, "PodInfo should be nil on error") - assert.Contains(t, err.Error(), "storage put error", "Error should contain storage error message") + // Test: pod with host network should return NotFound (shouldHandlePod returns false) + t.Run("pod with host network should return NotFound", func(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + Spec: corev1.PodSpec{ + NodeName: "test-node", + HostNetwork: true, + }, + } + + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(pod).Build() + mockStore := &mockStorage{data: make(map[string]interface{})} + + k8sObj := &k8s{ + client: fakeClient, + storage: mockStore, + mode: daemon.ModeENIMultiIP, + enableErdma: false, + statefulWorkloadKindSet: sets.New[string](), + nodeName: "test-node", + } + + podInfo, err := k8sObj.GetPod(context.Background(), "default", "test-pod", true) + + assert.Error(t, err, "Should return error for pod with host network") + assert.True(t, apierrors.IsNotFound(err), "Error should be NotFound") + assert.Nil(t, podInfo, "PodInfo should be nil") + }) + + // Test: pod with ignore label should return NotFound (shouldHandlePod returns false) + t.Run("pod with ignore label should return NotFound", func(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + Labels: map[string]string{ + types.IgnoreByTerway: "true", + }, + }, + Spec: corev1.PodSpec{ + NodeName: "test-node", + }, + } + + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(pod).Build() + mockStore := &mockStorage{data: make(map[string]interface{})} + + k8sObj := &k8s{ + client: fakeClient, + storage: mockStore, + mode: daemon.ModeENIMultiIP, + enableErdma: false, + statefulWorkloadKindSet: sets.New[string](), + nodeName: "test-node", + } + + podInfo, err := k8sObj.GetPod(context.Background(), "default", "test-pod", true) + + assert.Error(t, err, "Should return error for pod with ignore label") + assert.True(t, apierrors.IsNotFound(err), "Error should be NotFound") + assert.Nil(t, podInfo, "PodInfo should be nil") + }) } // ============================================================================== @@ -2063,3 +2213,8 @@ func TestSerialize(t *testing.T) { _, err = deserialize(v) assert.NoError(t, err) } + +func TestApiErr(t *testing.T) { + err := apierrors.NewNotFound(corev1.Resource("pod"), "test-pod") + require.True(t, apierrors.IsNotFound(err)) +} diff --git a/pkg/k8s/mocks/Kubernetes.go b/pkg/k8s/mocks/Kubernetes.go index 182cb297..cc3803a7 100644 --- a/pkg/k8s/mocks/Kubernetes.go +++ b/pkg/k8s/mocks/Kubernetes.go @@ -11,6 +11,8 @@ import ( mock "github.com/stretchr/testify/mock" + rest "k8s.io/client-go/rest" + types "github.com/AliyunContainerService/terway/types" v1 "k8s.io/api/core/v1" @@ -147,6 +149,26 @@ func (_m *Kubernetes) GetPod(ctx context.Context, namespace string, name string, return r0, r1 } +// GetRestConfig provides a mock function with no fields +func (_m *Kubernetes) GetRestConfig() *rest.Config { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for GetRestConfig") + } + + var r0 *rest.Config + if rf, ok := ret.Get(0).(func() *rest.Config); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*rest.Config) + } + } + + return r0 +} + // GetServiceCIDR provides a mock function with no fields func (_m *Kubernetes) GetServiceCIDR() *types.IPNetSet { ret := _m.Called() diff --git a/pkg/link/veth_test.go b/pkg/link/veth_test.go index 3f3b082f..53677076 100644 --- a/pkg/link/veth_test.go +++ b/pkg/link/veth_test.go @@ -81,20 +81,20 @@ func TestVethNameForPod_Extended(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := VethNameForPod(tt.podName, tt.namespace, tt.ifName, tt.prefix) - + if (err != nil) != tt.wantErr { t.Errorf("VethNameForPod() error = %v, wantErr %v", err, tt.wantErr) return } - + if !tt.wantErr { // Verify the result starts with the prefix assert.True(t, len(got) > len(tt.prefix), "Generated veth name should be longer than prefix") assert.True(t, got[:len(tt.prefix)] == tt.prefix, "Generated veth name should start with prefix") - + // Verify the total length is within limits (15 characters max for veth names) assert.True(t, len(got) <= 15, "Generated veth name should not exceed 15 characters") - + // Verify consistent generation for same inputs got2, err2 := VethNameForPod(tt.podName, tt.namespace, tt.ifName, tt.prefix) assert.NoError(t, err2) @@ -108,10 +108,10 @@ func TestVethNameForPod_EthZeroHandling(t *testing.T) { // Test that eth0 is treated the same as empty string name1, err1 := VethNameForPod("test-pod", "default", "eth0", "veth") assert.NoError(t, err1) - + name2, err2 := VethNameForPod("test-pod", "default", "", "veth") assert.NoError(t, err2) - + assert.Equal(t, name1, name2, "eth0 interface should be treated same as empty string") } @@ -121,7 +121,7 @@ func TestVethNameForPod_Consistency(t *testing.T) { namespace := "test-ns" ifName := "eth0" prefix := "test" - + // Generate multiple times and ensure consistency results := make([]string, 5) for i := 0; i < 5; i++ { @@ -129,7 +129,7 @@ func TestVethNameForPod_Consistency(t *testing.T) { assert.NoError(t, err) results[i] = result } - + // All results should be identical for i := 1; i < len(results); i++ { assert.Equal(t, results[0], results[i], "VethNameForPod should produce consistent results") diff --git a/pkg/utils/os_test.go b/pkg/utils/os_test.go index 60d2c748..cbd77732 100644 --- a/pkg/utils/os_test.go +++ b/pkg/utils/os_test.go @@ -19,4 +19,4 @@ func TestIsWindowsOS_LinuxEnvironment(t *testing.T) { if runtime.GOOS == "linux" { assert.False(t, IsWindowsOS()) } -} \ No newline at end of file +} diff --git a/pkg/utils/path_test.go b/pkg/utils/path_test.go index 26f227ba..390a5573 100644 --- a/pkg/utils/path_test.go +++ b/pkg/utils/path_test.go @@ -9,27 +9,27 @@ import ( func TestNormalizePath(t *testing.T) { tests := []struct { - name string - path string - expected string + name string + path string + expected string isWindows bool }{ { - name: "Unix absolute path on Linux", - path: "/usr/local/bin", - expected: "/usr/local/bin", + name: "Unix absolute path on Linux", + path: "/usr/local/bin", + expected: "/usr/local/bin", isWindows: false, }, { - name: "Unix relative path on Linux", - path: "relative/path", - expected: "relative/path", + name: "Unix relative path on Linux", + path: "relative/path", + expected: "relative/path", isWindows: false, }, { - name: "Windows style path on Linux", - path: "C:\\Windows\\System32", - expected: "C:\\Windows\\System32", + name: "Windows style path on Linux", + path: "C:\\Windows\\System32", + expected: "C:\\Windows\\System32", isWindows: false, }, } @@ -73,4 +73,4 @@ func TestMustGetWindowsSystemDrive_NonWindows(t *testing.T) { result := mustGetWindowsSystemDrive() assert.Equal(t, "", result) } -} \ No newline at end of file +} diff --git a/tests/utils/node_test.go b/tests/utils/node_test.go index 2ef7aa6f..39dbfb38 100644 --- a/tests/utils/node_test.go +++ b/tests/utils/node_test.go @@ -71,4 +71,3 @@ func TestDeploymentWithLingjunToleration(t *testing.T) { t.Error("Lingjun toleration not found in deployment") } } -