Skip to content
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

clean nginx temp configs function does not work #11568

Closed
ipaqsa opened this issue Jul 8, 2024 · 3 comments · Fixed by #11569
Closed

clean nginx temp configs function does not work #11568

ipaqsa opened this issue Jul 8, 2024 · 3 comments · Fixed by #11569
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@ipaqsa
Copy link
Contributor

ipaqsa commented Jul 8, 2024

What happened:
The testTemplate function creates a temp config(os.CreateTemp(tmpDir, tempNginxPattern)) in the tempDir/nginx directory (tmpDir := os.TempDir() + "/nginx") but the cleanTempNginxCfg function cleans only the tmp directory. This means that this function does not work, and wrong nginx temp configs are not deleted, and it leads to clogging up FS. And there is a wrong test, because the test creates files in the temp dir, and that is why it passes now.

Mistake is this function:

	err := filepath.Walk(os.TempDir(), func(path string, info os.FileInfo, err error) error {
		if err != nil {
			return err
		}
		if info.IsDir() && os.TempDir() != path {
			return filepath.SkipDir
		}

		dur, err := time.ParseDuration("-5m")
		if err != nil {
			return err
		}
		fiveMinutesAgo := time.Now().Add(dur)
		if strings.HasPrefix(info.Name(), tempNginxPattern) && info.ModTime().Before(fiveMinutesAgo) {
			files = append(files, path)
		}
		return nil
	})

Due to this condition the function checks only the temp dir and does not check nested tempDir/nginx

if info.IsDir() && os.TempDir() != path {
   return filepath.SkipDir
}

What you expected to happen:
Wrong nginx temp configs are deleted.

How to reproduce this issue:
Create nginx temp configs(files whose name begins with prefix nginx-cfg) in tempDir/nginx and wait 6 minutes, nothing will happen.

@ipaqsa ipaqsa added the kind/bug Categorizes issue or PR as related to a bug. label Jul 8, 2024
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jul 8, 2024
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link

github-actions bot commented Aug 8, 2024

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

@github-actions github-actions bot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Aug 8, 2024
@clementnuss
Copy link

I have just been impacted with this. would be nice if the PR got merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

Successfully merging a pull request may close this issue.

3 participants