Skip to content

Conversation

weyfonk
Copy link
Contributor

@weyfonk weyfonk commented Jul 28, 2025

The Fleet agent must be able to properly handle SIGTERM, which amounts to exiting with error code 0 (instead of 1) when its context is canceled.

Refers to #3919.

Additional Information

Checklist

- [ ] I have updated the documentation via a pull request in the
fleet-docs repository.

@weyfonk weyfonk requested a review from a team as a code owner July 28, 2025 13:58
Copy link
Collaborator

@thardeck thardeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a test for that, since the issue was a regression?

@weyfonk
Copy link
Contributor Author

weyfonk commented Jul 29, 2025

Should we add a test for that, since the issue was a regression?

Good shout, I've added end-to-end tests in a separate commit (see e2e/single-cluster/signals_test.go).

@weyfonk weyfonk requested a review from thardeck July 29, 2025 10:05
weyfonk added 2 commits July 29, 2025 14:02
The Fleet agent must be able to properly handle `SIGTERM`, which amounts
to exiting with error code 0 (instead of 1) when its context is
canceled.
New end-to-end test cases for SIGTERM handling should uncover possible
future regressions such as the one fixed by the previous commit.
@weyfonk weyfonk force-pushed the 3851-agent-signal-handling branch from 8113b5e to b909388 Compare July 29, 2025 12:02
weyfonk added 2 commits July 29, 2025 14:59
That configuration is consistent with what `dev/setup-fleet` does, which
is itself aligned on Rancher behaviour.
Some signal handling test cases would fail because running `kubectl
exec` against a deployment name would not necessarily run the command in
that same deployment based on an exact name match.
To work around this, we now compute pod names for the right combination
of labels, including shard IDs, and run `kubectl exec` directly against
the resolved pod.
@weyfonk weyfonk force-pushed the 3851-agent-signal-handling branch from b909388 to 1e2814a Compare July 29, 2025 13:19
@weyfonk weyfonk merged commit 1b64e8e into rancher:main Jul 29, 2025
14 checks passed
weyfonk added a commit to weyfonk/fleet that referenced this pull request Jul 29, 2025
The Fleet agent must be able to properly handle `SIGTERM`, which amounts
to exiting with error code 0 (instead of 1) when its context is
canceled.

New end-to-end test cases for SIGTERM handling should uncover possible
future regressions such as the one fixed by the previous commit.

* Deploy the Fleet agent into namespace `cattle-fleet-local-system`

That configuration is consistent with what `dev/setup-fleet` does, which
is itself aligned on Rancher behaviour.
thardeck pushed a commit that referenced this pull request Jul 30, 2025
The Fleet agent must be able to properly handle `SIGTERM`, which amounts
to exiting with error code 0 (instead of 1) when its context is
canceled.

New end-to-end test cases for SIGTERM handling should uncover possible
future regressions such as the one fixed by the previous commit.

* Deploy the Fleet agent into namespace `cattle-fleet-local-system`

That configuration is consistent with what `dev/setup-fleet` does, which
is itself aligned on Rancher behaviour.
@kkaempf kkaempf added this to the v2.13.0 milestone Aug 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants