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

Delete FluentPVCBinding if it times out without becoming Ready condition #39

Merged
merged 5 commits into from
Jul 5, 2024

Conversation

R-HNF
Copy link
Contributor

@R-HNF R-HNF commented Jun 26, 2024

Which issue(s) this PR fixes:

Fixes #38

What this PR does / why we need it:

Why
If FluentPVCBinding cannot find the target pod within one hour, it remains in Unknown status as an unnecessary resource in Kubernetes cluster. Such unnecessary resources can potentially impact the cluster and operations adversely, so I will modify it to be deleted. I added a process to delete PVC for the same reason.

Changes

  • Adding PodMissing condition.
  • If FluentPVCBinding cannot find the Pod and the FluentPVCBinding does not become the Ready condition within one hour, instead of setting the it to Unknown status, it will be deleted.
  • When PVC controller detects that FluentPVCBinding has become PodMissing, the PVC will proceed with deletion. Subsequently, FluentPVCBinding controller confirms that the PVC has disappeared and proceeds to delete the FluentPVCBinding.

How test it:

I have prepared a branch with applied the fixes and the reproduction method, so please check it here.

In addition, I have also made modifications to ensure compatibility with Apple Silicon (arm chip) in the branch. If you're curious about the changes for Apple Silicon, please try the reproduction method on an AMD-based PC.

Copy link

@banatt banatt left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link

@kei-gnu kei-gnu left a comment

Choose a reason for hiding this comment

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

LGTM!

@R-HNF
Copy link
Contributor Author

R-HNF commented Jul 5, 2024

@banatt @niqniqniqq @kei-gnu
Thank you for your review! I'll merge it!

@R-HNF R-HNF merged commit 4708da2 into st-tech:main Jul 5, 2024
3 checks passed
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.

FluentPVCBinding remains as Unknown status
4 participants