Skip to content

Conversation

AnushaY1916
Copy link
Contributor

No description provided.

Signed-off-by: AnushaY1916 <[email protected]>
Signed-off-by: AnushaY1916 <[email protected]>
Copy link
Collaborator

@dileepds dileepds left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: AnushaY1916 <[email protected]>
Copy link
Collaborator

@datamattsson datamattsson left a comment

Choose a reason for hiding this comment

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

Are we leaving residual serviceAccounts, roles and roleBindings around? I can't see these being pruned in deleteNFSResources.

livenessProbe := &core_v1.Probe{
ProbeHandler: core_v1.ProbeHandler{
Exec: &core_v1.ExecAction{
Command: []string{"/bin/sh", "/nfsHealthCheck.sh", "3", name, nfsNamespace},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we maintain a hash of actions for the 1, 2 and 3 in the script instead? I find this numbering very counter-intuitive. nfsNamespace is not being used in the script and should be removed.

Copy link
Collaborator

@datamattsson datamattsson left a comment

Choose a reason for hiding this comment

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

LGTM, can we please work #437 next?

@dileepds dileepds merged commit e21ffa7 into hpe-storage:master Apr 22, 2025
1 check 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.

4 participants