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
ceph: skip osd prepare job creation if osd daemon exists for the pvc #4277
Conversation
pkg/operator/ceph/cluster/osd/osd.go
Outdated
//Skip OSD prepare if deployement already exists for the PVC | ||
listOpts := metav1.ListOptions{LabelSelector: fmt.Sprintf("%s=%s,%s=%s", | ||
k8sutil.AppAttr, AppName, | ||
pvcLabelKey, osdProps.crushHostname, |
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 the crushHostName set above as the PVC name above on line 253, but I'd suggest we set this to volume.PersistentVolumeClaimSource.ClaimName
just to be clear that we want the pvc name and not the host name, just for code readability. The crushHostname is actually not the pvc name if the osd is not portable, this must be updated somewhere else.
pkg/operator/ceph/cluster/osd/osd.go
Outdated
return osds, fmt.Errorf("failed to get required osdInfo. %+v", osd) | ||
} | ||
|
||
osds = append(osds, osd) |
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.
nit: instead of appending, you could do this: return []OSDInfo{osd}, nil
@@ -828,3 +856,38 @@ func (c *Cluster) getPVCHostName(pvcName string) (string, error) { | |||
} | |||
return "", err | |||
} | |||
|
|||
func getOSDInfo(d *apps.Deployment) ([]OSDInfo, error) { |
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.
how about a unit test for this method?
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.
added unit test.
When an orchestration is restarted or retriggered and starts new osd prepare pods, they are unable to start because the device is already in use by the osd daemon pods. This fix simple skips the osd prepare pod creation if the daemon is already running. Signed-off-by: Santosh Pillai <sapillai@redhat.com>
ceph: skip osd prepare job creation if osd daemon exists for the pvc (bp #4277)
When an orchestration is restarted or retriggered and starts new osd prepare pods, they are unable to start because the device is already in use by the osd daemon pods.
This fix skips the osd prepare pod creation if the daemon is already running.
Signed-off-by: Santosh Pillai sapillai@redhat.com
Description of your changes:
Which issue is resolved by this Pull Request:
Resolves #
Checklist:
make codegen
) has been run to update object specifications, if necessary.