test: refactor container_kill_linux_test.go to use Tigron

[microness]

related: #4613

[ChengyuZhu6]

@microness I found the refactoring dropped a check "iptables rule removed after kill". Would you mind adding the post-kill validation?

[microness]

Thanks for the comment! Fixed.

[haytok]

IMO, instead of hard-coding the port number, it is better to use the port number acquired via portlock.Acquire(). So, could you rewrite it?

• https://github.com/containerd/nerdctl/blob/main/pkg/testutil/portlock/portlock.go

[microness]

Thanks for the comment! Fixed.

[haytok]

Can we re-write using nerdtest.Rootless ?

• https://github.com/containerd/nerdctl/blob/main/docs/testing/tools.md#requirements

[microness]

Thanks for the comment! Fixed.

[haytok]

Could you add logic to release the acquired port number?

• https://github.com/containerd/nerdctl/blob/main/pkg/testutil/portlock/portlock.go#L24
• https://github.com/containerd/nerdctl/blob/main/pkg/testutil/portlock/portlock.go#L59

		if p := data.Labels().Get("hostPort"); p != "" {
			if port, err := strconv.Atoi(p); err == nil {
				_ = portlock.Release(port)
			}
		}

[haytok]

Testing it within the Setup seems odd—couldn't we implement it using helper.Custom?

[microness]

Thanks for the comments!

I’ve fixed on your feedback and have a few questions.
I’d appreciate a re-review.

Q1. Using helpers.Custom in Setup

I also felt that putting assert.Assert directly in Setup was a bit awkward,
So I refactored it with helpers.Custom as suggested below.

// Description: "iptables forwarding rule should exist before container is killed"

// Setup:
helpers.Custom(
	"iptables",
	"-t", "nat",
	"-S", chain,
).Run(&test.Expected{
	ExitCode: expect.ExitCodeSuccess,
	Output: func(stdout string, t tig.T) {
		assert.Assert(t, strings.Contains(stdout, fmt.Sprintf("--dport %d", hostPort)))
		assert.Assert(t, strings.Contains(stdout, "DNAT"))
		assert.Assert(t, strings.Contains(stdout, containerIP))
	},
})

According to the documentation, helpers can be used in all phases including Setup and Cleanup.

nerdctl/mod/tigron/test/interfaces.go

Lines 70 to 72 in 30bc993

	// Helpers provides a set of helpers to run commands with simple expectations,
	// available at all stages of a test (Setup, Cleanup, etc...)
	type Helpers interface {

Does that mean using helpers.Custom in Setup is acceptable in your view?

Q2. Using iptablesutil.ForwardExists vs helpers.Custom with iptables command

For the same reason, I replaced iptablesutil.ForwardExists with a helpers.Custom with iptables command.
Do you think this is a better fit for Setup, or would you prefer keeping the iptablesutil?

Thanks again for the review, I learned a lot.