Skip to content

error: "failed to check application" and "failed to clean partially cloned repository" #5338

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.

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

Open
peaceiris opened this issue Nov 14, 2024 · 3 comments
Labels
kind/bug Something isn't working as expected

Comments

@peaceiris
Copy link
Contributor

peaceiris commented Nov 14, 2024

What happened:

The following errors appear when a piped ends:

log

The error failed to clean partially cloned repository seems related to the process implemented in the pull request below. The content of the error field is err: context canceled, out: .

The error failed to check application: ... has two possible forms:

  • error: failed to load new manifests: unable to run kustomize template: signal: killed
  • error: failed to load new manifests: unable to run kustomize template: context canceled

What you expected to happen:

A piped completes shutdown without those errors.

How to reproduce it:

Environment:

  • piped version: v0.49.0 (at least) to v0.49.3
  • control-plane version:
  • Others:
@peaceiris peaceiris added the kind/bug Something isn't working as expected label Nov 14, 2024
@peaceiris peaceiris changed the title "failed to check application" and "failed to clean partially cloned repository" error: "failed to check application" and "failed to clean partially cloned repository" Nov 14, 2024
@abhinavs1920
Copy link

Hiii, @peaceiris @t-kikuc

What I can find out is that errors seem to be caused by context.Canceled during shutdown, especially after the git clean logic added in PR #5312. A good fix would be to skip or silently ignore cleanup and manifest-loading steps when ctx.Err() != nil, since these are expected cancellations during normal shutdown.

I can open a PR to handle this with proper checks.

Thankss!!!

@t-kikuc
Copy link
Member

t-kikuc commented May 22, 2025

@Warashi Please help us when you have time 🙏

@Warashi
Copy link
Member

Warashi commented May 22, 2025

Hi, @abhinavs1920
Thank you for the investigation.

I think cleanup operations should be executed even when the context is canceled.
We can run the piped in a container, a VM, or a bare metal machine. In the VM or bare metal environment, the files created by the piped should be cleaned when the piped is shut down.

We should check whether we can ignore the error even if the error comes from the context cancellation.

In this case, we can ignore the error because the operation is a kind of cleanup but partially clean, not entirely. The entire cleanup will executed in other codes.

Additionally, the issue shows not only the error failed to clean partially cloned repository. The screenshot shows another error message: failed to check application.

This may come from here; we can ignore this when the context is canceled because it's not a cleanup operation.

if err := d.checkApplication(ctx, app, gitRepo, headCommit); err != nil {
d.logger.Error(fmt.Sprintf("failed to check application: %s", app.Id), zap.Error(err))
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

No branches or pull requests

4 participants