Skip to content

Use tini-git image for the resolvers deployment #8895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
tekton-robot merged 1 commit into from
Jul 29, 2025
Merged

Use tini-git image for the resolvers deployment #8895

tekton-robot merged 1 commit into from
Jul 29, 2025

Conversation

@vdemeester
Copy link
Member

@vdemeester vdemeester commented Jul 18, 2025 •
edited
Loading

Changes

And make sure the entrypoint (command) is tini.

Closes #8830
/kind bug

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
  • Has Tests included if any functionality added or changed
  • pre-commit Passed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including functionality, content, code)
  • Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
  • Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
  • Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

The resolvers deployment now uses the `tini` init system to make sure we don't end up with a lot of git process zombies.

@tekton-robot tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. labels Jul 18, 2025
@vdemeester vdemeester added this to the v1.3.0 (LTS) milestone Jul 18, 2025
@tekton-robot tekton-robot requested review from dibyom and jerop July 18, 2025 14:28
@tekton-robot tekton-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 18, 2025
@vdemeester
Copy link
Member Author

vdemeester commented Jul 18, 2025

cc @aThorp96

@vdemeester vdemeester force-pushed the resolver-use-tini-git branch 2 times, most recently from a2db1b1 to e2c86db Compare July 21, 2025 09:44
@tekton-robot tekton-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 21, 2025
aThorp96
aThorp96 suggested changes Jul 21, 2025
View reviewed changes
Copy link
Member

@aThorp96 aThorp96 left a comment

Choose a reason for hiding this comment

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

The Performance settings in the operator overwrite the args/CMD, so the resolvers binary must be part of the command/ENTRYPOINT instead of the args

config/resolvers/resolvers-deployment.yaml
Comment on lines 67 to 78
command:
- /sbin/tini
- --
args:
- /ko-app/resolvers
Copy link
Member

@aThorp96 aThorp96 Jul 21, 2025

Choose a reason for hiding this comment

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

Suggested change
command:
- /sbin/tini
- --
args:
- /ko-app/resolvers
command:
- /sbin/tini
- --
- /ko-app/resolvers
args: []

Copy link
Member

@aThorp96 aThorp96 Jul 21, 2025

Choose a reason for hiding this comment

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

The other side of this is: maybe it's a bug that the performance settings in the Operator overwrite the args instead of appending to them 🤔

Copy link
Member Author

@vdemeester vdemeester Jul 21, 2025

Choose a reason for hiding this comment

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

ah the override, not "append" ?

Copy link
Member

@aThorp96 aThorp96 Jul 21, 2025

Choose a reason for hiding this comment

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

Unfortunately it's override 😞

@vdemeester
Copy link
Member Author

vdemeester commented Jul 21, 2025

May require tektoncd/plumbing#2708 (but not absolutely necessary for now).

@vdemeester vdemeester force-pushed the resolver-use-tini-git branch from e2c86db to a32cc8a Compare July 28, 2025 12:41
aThorp96
aThorp96 reviewed Jul 28, 2025
View reviewed changes
Copy link
Member

@aThorp96 aThorp96 left a comment

Choose a reason for hiding this comment

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

I think the image needs to be updated in the presubmit tests as well

@vdemeester
Copy link
Member Author

vdemeester commented Jul 28, 2025

I think the image needs to be updated in the presubmit tests as well

oh boy.. some many places...

@vdemeester
Use tini-git image for the resolvers deployment
0743ed7 
And make sure the entrypoint (command) is tini.

Signed-off-by: Vincent Demeester <[email protected]>
@vdemeester vdemeester force-pushed the resolver-use-tini-git branch from a32cc8a to 0743ed7 Compare July 28, 2025 13:50
afrittoli
afrittoli reviewed Jul 28, 2025
View reviewed changes
Copy link
Member

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

/approve

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 28, 2025
@vdemeester
Copy link
Member Author

vdemeester commented Jul 28, 2025

/retest

aThorp96
aThorp96 approved these changes Jul 29, 2025
View reviewed changes
Copy link
Member

@aThorp96 aThorp96 left a comment

Choose a reason for hiding this comment

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

/lgtm 🙏🏼

@waveywaves
Copy link
Member

waveywaves commented Jul 29, 2025

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2025
@tekton-robot
Copy link
Collaborator

tekton-robot commented Jul 29, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli, aThorp96, waveywaves

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [afrittoli,waveywaves]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot merged commit ed6506b into tektoncd:main Jul 29, 2025
49 of 50 checks passed
@vdemeester vdemeester deleted the resolver-use-tini-git branch July 29, 2025 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@afrittoli afrittoli afrittoli left review comments

@waveywaves waveywaves waveywaves approved these changes

@dibyom dibyom Awaiting requested review from dibyom

@jerop jerop Awaiting requested review from jerop

+1 more reviewer

@aThorp96 aThorp96 aThorp96 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@waveywaves waveywaves

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

Tekton Community Roadmap
Status: Done

Milestone

v1.3.0 (LTS)

Development

Successfully merging this pull request may close these issues.

Git Resolver - git binary not reaping zombie processes

5 participants

@vdemeester @waveywaves @tekton-robot @afrittoli @aThorp96