-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
nfs: fix nfs grace period when multus is enabled #11110
Conversation
6b422db
to
af13194
Compare
This pull request has merge conflicts that must be resolved before it can be merged. @BlaineEXE please rebase it. https://rook.io/docs/rook/latest/Contributing/development-flow/#updating-your-fork |
af13194
to
5575b76
Compare
Run the ganesha-rados-grace command in a remote pod when multus networking is enabled. Signed-off-by: parth-gr <paarora@redhat.com> Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>
5575b76
to
51fac0a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me I see NFS pod are in healthy state in the CI,
Just some doubts
@@ -179,7 +205,7 @@ func (c *CephToolCommand) run() ([]byte, error) { | |||
|
|||
// NewRBDCommand does not use the --out-file option so we only check for remote execution here | |||
// Still forcing the check for the command if the behavior changes in the future | |||
if command == RBDTool || command == RadosTool { | |||
if command == RBDTool || command == RadosTool || command == GaneshaRadosGraceTool { | |||
if c.RemoteExecution { | |||
output, stderr, err = c.context.RemoteExecutor.ExecCommandInContainerWithFullOutputWithTimeout(c.clusterInfo.Context, ProxyAppLabel, CommandProxyInitContainerName, c.clusterInfo.Namespace, append([]string{command}, args...)...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem was because the cmd was not called by the remote executor
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct.
|
||
return r.context.Executor.ExecuteCommandWithEnv(env, cmd, args...) | ||
cmd := cephclient.NewGaneshaRadosGraceCommand(r.context, r.clusterInfo, args) | ||
_, err := cmd.RunWithTimeout(exec.CephCommandsTimeout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a lot of cmds running by context.Executor
why this needs special attention??
@@ -167,7 +193,7 @@ func (c *CephToolCommand) run() ([]byte, error) { | |||
} else { | |||
// the `rbd` tool doesn't use special flag for plain format | |||
switch c.tool { | |||
case RBDTool, RadosTool: | |||
case RBDTool, RadosTool, GaneshaRadosGraceTool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is run
called by GaneshaRadosGraceTool cmd??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the new design, NewGaneshaRadosGrace() creates a command that, when run, goes through this path. The old command didn't.
nfs: fix nfs grace period when multus is enabled (backport #11110)
Run the ganesha-rados-grace command in a remote pod when multus networking is enabled.
Signed-off-by: parth-gr paarora@redhat.com
Signed-off-by: Blaine Gardner blaine.gardner@redhat.com
Description of your changes:
Which issue is resolved by this Pull Request:
Resolves #11039
Checklist:
skip-ci
on the PR.