Skip to content

fix: pinger pod crash on one-shot startup#4749

Open
Ivo-Yankov wants to merge 9 commits into
mainfrom
04354-fix-crashing-pinger
Open

fix: pinger pod crash on one-shot startup#4749
Ivo-Yankov wants to merge 9 commits into
mainfrom
04354-fix-crashing-pinger

Conversation

@Ivo-Yankov

Copy link
Copy Markdown
Contributor

Description

This PR fixes three small issues:

  • The Pinger pod crashes when one-shot is deployed in parallel mode. This PR fixes it by enabling the Pinger only after the network nodes have started.

  • The PR also fixes an issue with CraneDependencyManager - on every one-shot command, even when the dependency exists locally, it makes an unnecessary request to the GitHub API, which may result in a 403 error due to GitHub's rate limiting.

  • Removed a wait condition of the Relay during one-shot. Since v0.77.0, it no longer requires the network node to be running before it gets deployed.

Related Issues

Pull request (PR) checklist

  • This PR added tests (unit, integration, and/or end-to-end)
  • This PR updated documentation
  • This PR added no TODOs or commented out code
  • This PR has no breaking changes
  • Any technical debt has been documented as a separate issue and linked to this PR
  • Any package.json changes have been explained to and approved by a repository manager
  • All related issues have been linked to this PR
  • All changes in this PR are included in the description
  • When this PR merges the commits will be squashed and the title will be used as the commit message, the 'commit message guidelines' below have been followed

Testing

  • This PR added unit tests
  • This PR added integration/end-to-end tests
  • These changes required manual testing that is documented below
  • Anything not tested is documented

The following manual testing was done:

  • TBD

The following was not tested:

  • TBD
Commit message guidelines We use 'Conventional Commits' to ensure that our commit messages are easy to read, follow a consistent format, and for automated release note generation. Please follow the guidelines below when writing your commit messages:
  1. BREAKING CHANGE: a commit that has a footer BREAKING CHANGE:, or appends a ! after the type/scope, introduces a breaking API change (correlating with MAJOR in Semantic Versioning). A BREAKING CHANGE can be part of commits of any type. NOTE: currently breaking changes will only bump the MAJOR version.
  2. The title is prefixed with one of the following:
Prefix Description Semantic Version Update Captured in Release Notes
feat: a new feature MINOR Yes
fix: a bug fix PATCH Yes
perf: performance PATCH Yes
refactor: code change that isn't feature or fix none No
test: adding missing tests none No
docs: changes to documentation none Yes
build: changes to build process none No
ci: changes to CI configuration none No
style: formatting, missing semi-colons, etc none No
chore: updating grunt tasks etc; no production code change none No

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
@Ivo-Yankov Ivo-Yankov self-assigned this Jun 19, 2026
@Ivo-Yankov Ivo-Yankov requested a review from a team as a code owner June 19, 2026 13:09
@trunk-io

trunk-io Bot commented Jun 19, 2026

Copy link
Copy Markdown

✨ Submitted to Merge by @Ivo-Yankov. It will be added to the merge queue once all branch protection rules pass and there are no merge conflicts with the target branch. See more details here.

@Ivo-Yankov Ivo-Yankov changed the title fix: pinger pod crashes once on one-shot startup fix: pinger pod crash once on one-shot startup Jun 19, 2026
@Ivo-Yankov Ivo-Yankov changed the title fix: pinger pod crash once on one-shot startup fix: pinger pod crash on one-shot startup Jun 19, 2026
@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Unit Test Results - Linux

38 tests  ±0   38 ✅ ±0   0s ⏱️ ±0s
17 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit cabc9c2. ± Comparison against base commit ee99636.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Unit Test Results - Windows

    1 files  ±0    337 suites  ±0   10s ⏱️ ±0s
1 068 tests ±0  1 068 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 072 runs  ±0  1 072 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 27e5721. ± Comparison against base commit 526ee3a.

♻️ This comment has been updated with latest results.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Test Report

 10 files   94 suites   1h 23m 27s ⏱️
301 tests 301 ✅ 0 💤 0 ❌
320 runs  320 ✅ 0 💤 0 ❌

Results for commit b7acf39.

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

E2E Test Report

 10 files  ±0   94 suites  ±0   1h 28m 6s ⏱️ + 4m 2s
302 tests ±0  302 ✅ ±0  0 💤 ±0  0 ❌ ±0 
321 runs  ±0  321 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit fe9c4b7. ± Comparison against base commit e8a8c90.

♻️ This comment has been updated with latest results.

@Ivo-Yankov Ivo-Yankov added the PR: Needs Team Approval A pull request that needs review from a team member. label Jun 19, 2026
}

await this.preInstall();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what are the reason or logic move up this preInstall step ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The preInstall method in the craneDependencyManager sends an API request to GitHub, which is redundant if isInstalledLocallyAndMeetsRequirements() === true.

JeffreyDallas
JeffreyDallas previously approved these changes Jun 19, 2026
@jeromy-cannon jeromy-cannon added P2-⏭️💎 Next Milestone & Goals P0-🔥 ASAP and removed P2-⏭️💎 Next Milestone & Goals labels Jun 19, 2026
jeromy-cannon
jeromy-cannon previously approved these changes Jun 19, 2026
jan-milenkov
jan-milenkov previously approved these changes Jun 22, 2026
@jan-milenkov jan-milenkov added PR: Ready to Merge A pull request that is ready to merge. and removed PR: Needs Team Approval A pull request that needs review from a team member. labels Jun 22, 2026
@jeromy-cannon jeromy-cannon marked this pull request as draft June 22, 2026 11:01
@jeromy-cannon jeromy-cannon added the PR: Merge Conflicts A pull request that has merge conflicts that need to be resolved. label Jun 22, 2026
# Conflicts:
#	src/commands/one-shot/orchestrator/deploy/default-one-shot-deploy-orchestrator.ts
@Ivo-Yankov Ivo-Yankov dismissed stale reviews from jan-milenkov and jeromy-cannon via 13678f6 June 22, 2026 12:49
@Ivo-Yankov Ivo-Yankov marked this pull request as ready for review June 22, 2026 13:32
@Ivo-Yankov Ivo-Yankov added PR: Needs Team Approval A pull request that needs review from a team member. and removed PR: Merge Conflicts A pull request that has merge conflicts that need to be resolved. labels Jun 22, 2026
@jeromy-cannon jeromy-cannon added PR: Merge Conflicts A pull request that has merge conflicts that need to be resolved. and removed PR: Needs Team Approval A pull request that needs review from a team member. labels Jun 23, 2026
@jeromy-cannon jeromy-cannon marked this pull request as draft June 23, 2026 05:56
# Conflicts:
#	src/commands/one-shot/orchestrator/deploy/default-one-shot-deploy-orchestrator.ts
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
@Ivo-Yankov Ivo-Yankov added PR: Needs Team Approval A pull request that needs review from a team member. and removed PR: Merge Conflicts A pull request that has merge conflicts that need to be resolved. labels Jun 23, 2026
@Ivo-Yankov Ivo-Yankov marked this pull request as ready for review June 24, 2026 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P0-🔥 ASAP PR: Needs Team Approval A pull request that needs review from a team member. PR: Ready to Merge A pull request that is ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mirror-1-pinger-… in CrashLoopBackOff on "successful" deploys

4 participants