Skip to content

cleanup gcsfuse errofile before next NPV attempt#1371

Open
Sneha-at wants to merge 1 commit into
GoogleCloudPlatform:mainfrom
Sneha-at:cleanup_error
Open

cleanup gcsfuse errofile before next NPV attempt#1371
Sneha-at wants to merge 1 commit into
GoogleCloudPlatform:mainfrom
Sneha-at:cleanup_error

Conversation

@Sneha-at
Copy link
Copy Markdown
Collaborator

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change

/kind bug

/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:
Sidecar container adds any errors it encounters to an error file. This file is cleaned up only before a new mount operation is attempted. Any errors between NPV retries are propagated to the next attempt causing Node Publish Volume to log failures even if it succeeds. This PR cleans up the error file once it is read in node.go thus ensuring the error file only contains results related to the latest NPV call.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Removes excessive logging caused by transient Node Publish Volume failures.

@google-oss-prow
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Sneha-at

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

Details Needs approval from an approver in each of these files:

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

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces logic to clear stale error files in the NodePublishVolume function, preventing subsequent retries from being blocked by previous failures. It also includes a new unit test, TestNodePublishVolumeErrorFileCleanup, to verify that the error file is correctly deleted and that subsequent attempts succeed. The reviewer suggested using the existing util.CheckAndDeleteStaleFile utility instead of manual file removal to improve code reuse and ensure consistent path handling.

Comment thread pkg/csi_driver/node.go Outdated
@amacaskill
Copy link
Copy Markdown
Collaborator

Is it possible to add an e2e test to make sure error file is cleared after it failed then succeeded after?

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.

2 participants