Skip to content

Commit 9539028

Browse files
BSWANGBSWANG
authored
Merge pull request #955 from l1b0k/main
enhance terminted pod handle
2 parents 955b492 + e9b3585 commit 9539028

File tree

18 files changed

+548
-445
lines changed

18 files changed

+548
-445
lines changed

‎AGENTS.md‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# Terway Project Development Guide
2+
3+
## Dev environment tips
4+
5+
- Use `make help` to see all available make targets and their descriptions
6+
- Run `make generate` to generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations
7+
- Use `make manifests` to generate CustomResourceDefinition objects
8+
- Run `make fmt` to format Go code with `go fmt`
9+
- Use `make vet` to run `go vet` against code with build tags
10+
11+
## Testing instructions
12+
13+
- Find the CI plan in the `.github/workflows` folder (check.yml, build.yml)
14+
- Run `make quick-test` to run all tests
15+
- Run `make lint` to run golangci-lint with the configuration in `.golangci.yml`
16+
- Run `make lint-fix` to run golangci-lint and perform automatic fixes
17+
- Fix any test or lint errors until the whole suite passes
18+
- After moving files or changing imports, run `make fmt` and `make vet` to ensure code quality
19+
- Add or update tests for the code you change, even if nobody asked
20+
21+
## PR instructions
22+
23+
- Title format: `[terway] <Title>` or `[component] <Title>`
24+
- Always run `make lint` and `make test` before committing
25+
- Ensure `go mod tidy` and `go mod vendor` are run before submitting (checked in CI)
26+
- Make sure all build tags are properly set (privileged, default_build)
27+
- Verify that your changes pass the GitHub Actions workflows:
28+
- Code formatting and linting (golangci-lint)
29+
- Unit tests with coverage
30+
- Module vendoring checks
31+
- Super-linter for markdown and bash files

‎Makefile‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ vet: ## Run go vet against code.
5353
test: manifests generate fmt vet envtest datapath-test## Run tests.
5454
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
5555

56+
.PHONY: test-quick
57+
test-quick: manifests generate fmt vet envtest ## Run tests.
58+
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
59+
5660
.PHONY: lint
5761
lint: golangci-lint ## Run golangci-lint linter & yamllint
5862
$(GOLANGCI_LINT) run

‎cmd/terway-cli/cmd_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import (
66
"testing"
77

88
"github.com/agiledragon/gomonkey/v2"
9-
"github.com/stretchr/testify/assert"
109
"github.com/spf13/cobra"
10+
"github.com/stretchr/testify/assert"
1111

1212
"github.com/AliyunContainerService/terway/pkg/aliyun/metadata"
1313
)
@@ -42,12 +42,12 @@ func TestPrintKV(t *testing.T) {
4242
func TestArgumentValidation(t *testing.T) {
4343
t.Run("runList argument validation", func(t *testing.T) {
4444
cmd := &cobra.Command{}
45-
45+
4646
// Test with exactly 2 arguments (should fail)
4747
err := runList(cmd, []string{"arg1", "arg2"})
4848
assert.Error(t, err)
4949
assert.Contains(t, err.Error(), "too many arguments")
50-
50+
5151
// Test with more than 2 arguments (should fail)
5252
err = runList(cmd, []string{"arg1", "arg2", "arg3"})
5353
assert.Error(t, err)
@@ -56,17 +56,17 @@ func TestArgumentValidation(t *testing.T) {
5656

5757
t.Run("runShow argument validation", func(t *testing.T) {
5858
cmd := &cobra.Command{}
59-
59+
6060
// Test with no arguments
6161
err := runShow(cmd, []string{})
6262
assert.Error(t, err)
6363
assert.Contains(t, err.Error(), "no arguments")
64-
64+
6565
// Test with 2 or more arguments
6666
err = runShow(cmd, []string{"type", "name"})
6767
assert.Error(t, err)
6868
assert.Contains(t, err.Error(), "too many arguments")
69-
69+
7070
// Test with 3 arguments
7171
err = runShow(cmd, []string{"type", "name", "extra"})
7272
assert.Error(t, err)
@@ -75,28 +75,28 @@ func TestArgumentValidation(t *testing.T) {
7575

7676
t.Run("runExecute argument validation", func(t *testing.T) {
7777
cmd := &cobra.Command{}
78-
78+
7979
// Test with no arguments
8080
err := runExecute(cmd, []string{})
8181
assert.Error(t, err)
8282
assert.Contains(t, err.Error(), "too few arguments")
83-
83+
8484
// Test with 1 argument
8585
err = runExecute(cmd, []string{"type"})
8686
assert.Error(t, err)
8787
assert.Contains(t, err.Error(), "too few arguments")
88-
88+
8989
// Test with 2 arguments
9090
err = runExecute(cmd, []string{"type", "name"})
9191
assert.Error(t, err)
9292
assert.Contains(t, err.Error(), "too few arguments")
93-
93+
9494
// Test with exactly 3 arguments should pass validation
9595
// We don't call runExecute here since it will access nil client
9696
// Instead, we verify the argument parsing logic
9797
args := []string{"type", "name", "command"}
9898
assert.True(t, len(args) >= 3, "Should have at least 3 arguments")
99-
99+
100100
// Verify argument assignment logic
101101
typ, name, command := args[0], args[1], args[2]
102102
remainingArgs := args[3:]
@@ -556,4 +556,4 @@ func TestRunMetadata_ErrorCases(t *testing.T) {
556556
err := runMetadata(cmd, []string{})
557557
assert.NoError(t, err)
558558
})
559-
}
559+
}

‎daemon/builder.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ func (b *NetworkServiceBuilder) PostInitForCRDV2() *NetworkServiceBuilder {
465465
if b.err != nil {
466466
return b
467467
}
468-
crdv2 := eni.NewCRDV2(b.service.k8s.NodeName(), b.namespace)
468+
crdv2 := eni.NewCRDV2(b.service.k8s.GetRestConfig(), b.service.k8s.NodeName(), b.namespace)
469469
mgr := eni.NewManager(nil, 0, []eni.NetworkInterface{crdv2}, daemon.EniSelectionPolicy(b.config.EniSelectionPolicy), nil)
470470

471471
svc := b.RunENIMgr(b.ctx, mgr)

‎daemon/daemon.go‎

Lines changed: 23 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ var serviceLog = logf.Log.WithName("server")
8686
var _ rpc.TerwayBackendServer = (*networkService)(nil)
8787

8888
// return resource relation in db, or return nil.
89-
func (n *networkService) getPodResource(info *daemon.PodInfo) (daemon.PodResources, error) {
90-
obj, err := n.resourceDB.Get(utils.PodInfoKey(info.Namespace, info.Name))
89+
func (n *networkService) getPodResource(podID string) (daemon.PodResources, error) {
90+
obj, err := n.resourceDB.Get(podID)
9191
if err == nil {
9292
return obj.(daemon.PodResources), nil
9393
}
@@ -98,9 +98,8 @@ func (n *networkService) getPodResource(info *daemon.PodInfo) (daemon.PodResourc
9898
return daemon.PodResources{}, err
9999
}
100100

101-
func (n *networkService) deletePodResource(info *daemon.PodInfo) error {
102-
key := utils.PodInfoKey(info.Namespace, info.Name)
103-
return n.resourceDB.Delete(key)
101+
func (n *networkService) deletePodResource(podID string) error {
102+
return n.resourceDB.Delete(podID)
104103
}
105104

106105
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
163162
}
164163

165164
// 2. Find old resource info
166-
oldRes, err := n.getPodResource(pod)
165+
oldRes, err := n.getPodResource(podID)
167166
if err != nil {
168167
return nil, &types.Error{
169168
Code: types.ErrInternalError,
@@ -331,30 +330,31 @@ func (n *networkService) ReleaseIP(ctx context.Context, r *rpc.ReleaseIPRequest)
331330
// 0. Get pod Info
332331
pod, err := n.k8s.GetPod(ctx, r.K8SPodNamespace, r.K8SPodName, true)
333332
if err != nil {
334-
if k8sErr.IsNotFound(err) {
335-
return reply, nil
333+
if !k8sErr.IsNotFound(err) {
334+
l.Error(err, "get pod failed")
335+
// ignore error, do not block delete
336336
}
337-
return nil, err
338337
}
339338

340339
cni := &daemon.CNI{
341340
PodName: r.K8SPodName,
342341
PodNamespace: r.K8SPodNamespace,
343342
PodID: podID,
344-
PodUID: pod.PodUID,
343+
}
344+
345+
var podIpStickTime time.Duration
346+
if pod != nil {
347+
cni.PodUID = pod.PodUID
348+
podIpStickTime = pod.IPStickTime
345349
}
346350

347351
// 1. Init Context
348352

349-
oldRes, err := n.getPodResource(pod)
353+
oldRes, err := n.getPodResource(podID)
350354
if err != nil {
351355
return nil, err
352356
}
353357

354-
if !n.verifyPodNetworkType(pod.PodNetworkType) {
355-
return nil, fmt.Errorf("unexpect pod network type allocate, maybe daemon mode changed: %+v", pod.PodNetworkType)
356-
}
357-
358358
if oldRes.ContainerID != nil {
359359
if r.K8SPodInfraContainerId != *oldRes.ContainerID {
360360
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)
365365
cni.PodUID = oldRes.PodInfo.PodUID
366366
}
367367

368-
if n.ipamType == types.IPAMTypeCRD || pod.IPStickTime == 0 {
368+
if n.ipamType == types.IPAMTypeCRD || podIpStickTime <= 0 {
369369
for _, resource := range oldRes.Resources {
370370
res := parseNetworkResource(resource)
371371
if res == nil {
@@ -379,7 +379,7 @@ func (n *networkService) ReleaseIP(ctx context.Context, r *rpc.ReleaseIPRequest)
379379
return nil, err
380380
}
381381
}
382-
err = n.deletePodResource(pod)
382+
err = n.deletePodResource(podID)
383383
if err != nil {
384384
return nil, fmt.Errorf("error delete pod resource: %w", err)
385385
}
@@ -388,6 +388,7 @@ func (n *networkService) ReleaseIP(ctx context.Context, r *rpc.ReleaseIPRequest)
388388
return reply, nil
389389
}
390390

391+
// GetIPInfo return cached alloc ip info
391392
func (n *networkService) GetIPInfo(ctx context.Context, r *rpc.GetInfoRequest) (*rpc.GetInfoReply, error) {
392393
podID := utils.PodInfoKey(r.K8SPodNamespace, r.K8SPodName)
393394
log := logf.FromContext(ctx)
@@ -409,27 +410,17 @@ func (n *networkService) GetIPInfo(ctx context.Context, r *rpc.GetInfoRequest) (
409410

410411
var err error
411412

412-
// 0. Get pod Info
413-
pod, err := n.k8s.GetPod(ctx, r.K8SPodNamespace, r.K8SPodName, true)
414-
if err != nil {
415-
return nil, &types.Error{
416-
Code: types.ErrInvalidArgsErrCode,
417-
Msg: err.Error(),
418-
R: err,
419-
}
420-
}
421-
422413
// 1. Init Context
423414
reply := &rpc.GetInfoReply{
424415
Success: true,
425416
IPv4: n.enableIPv4,
426417
IPv6: n.enableIPv6,
427418
}
428419

429-
switch pod.PodNetworkType {
430-
case daemon.PodNetworkTypeENIMultiIP:
420+
switch n.daemonMode {
421+
case daemon.ModeENIMultiIP:
431422
reply.IPType = rpc.IPType_TypeENIMultiIP
432-
case daemon.PodNetworkTypeVPCENI:
423+
case daemon.ModeENIOnly:
433424
reply.IPType = rpc.IPType_TypeVPCENI
434425
default:
435426
return nil, &types.Error{
@@ -439,7 +430,7 @@ func (n *networkService) GetIPInfo(ctx context.Context, r *rpc.GetInfoRequest) (
439430
}
440431

441432
// 2. Find old resource info
442-
oldRes, err := n.getPodResource(pod)
433+
oldRes, err := n.getPodResource(podID)
443434
if err != nil {
444435
return nil, &types.Error{
445436
Code: types.ErrInternalError,
@@ -448,12 +439,6 @@ func (n *networkService) GetIPInfo(ctx context.Context, r *rpc.GetInfoRequest) (
448439
}
449440
}
450441

451-
if !n.verifyPodNetworkType(pod.PodNetworkType) {
452-
return nil, &types.Error{
453-
Code: types.ErrInvalidArgsErrCode,
454-
Msg: "Unexpected network type, maybe daemon mode changed",
455-
}
456-
}
457442
if oldRes.ContainerID != nil {
458443
if r.K8SPodInfraContainerId != *oldRes.ContainerID {
459444
log.Info("cni request not match stored resource, ignored", "old", *oldRes.ContainerID)
@@ -640,7 +625,7 @@ func (n *networkService) gcPods(ctx context.Context) error {
640625
}
641626
}
642627

643-
err = n.deletePodResource(podRes.PodInfo)
628+
err = n.deletePodResource(podID)
644629
if err != nil {
645630
return err
646631
}

0 commit comments

Comments
 (0)