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.

Sign up for GitHub

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

Jump to bottom

ceph: fix panic when recreating the csidriver object #8582

Merged
merged 3 commits into from Aug 24, 2021
+18 −18
Merged

ceph: fix panic when recreating the csidriver object #8582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d48f67a
ceph: fix panic in reCreateCSIDriverInfo
Madhu-1 Aug 24, 2021
e2dc418
ceph: fix logging of csidriver
Madhu-1 Aug 24, 2021
d246dde
ceph: remove variable declaration
Madhu-1 Aug 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 8 additions & 8 deletions pkg/operator/ceph/csi/betav1csidriver.go
View file
Open in desktop
Expand Up @@ -72,6 +72,8 @@ func (d beta1CsiDriver) createCSIDriverInfo(ctx context.Context, clientset kuber
// As FSGroupPolicy field is immutable, should be set only during create time.
// if the request is to change the FSGroupPolicy, we are deleting the CSIDriver object and creating it.
if driver.Spec.FSGroupPolicy != nil && csiDriver.Spec.FSGroupPolicy != nil && *driver.Spec.FSGroupPolicy != *csiDriver.Spec.FSGroupPolicy {
d.csiClient = csidrivers
Yuggupta27 marked this conversation as resolved.
Show resolved Hide resolved
d.csiDriver = csiDriver
return d.reCreateCSIDriverInfo(ctx)
}

Expand All @@ -88,18 +90,16 @@ func (d beta1CsiDriver) createCSIDriverInfo(ctx context.Context, clientset kuber
}

func (d beta1CsiDriver) reCreateCSIDriverInfo(ctx context.Context) error {
csiDriver := d.csiDriver
csiClient := d.csiClient
err := csiClient.Delete(ctx, csiDriver.Name, metav1.DeleteOptions{})
err := d.csiClient.Delete(ctx, d.csiDriver.Name, metav1.DeleteOptions{})
if err != nil {
return errors.Wrapf(err, "failed to delete CSIDriver object for driver %q", csiDriver.Name)
return errors.Wrapf(err, "failed to delete CSIDriver object for driver %q", d.csiDriver.Name)
}
logger.Infof("CSIDriver object deleted for driver %q", csiDriver.Name)
_, err = csiClient.Create(ctx, csiDriver, metav1.CreateOptions{})
logger.Infof("CSIDriver object deleted for driver %q", d.csiDriver.Name)
_, err = d.csiClient.Create(ctx, d.csiDriver, metav1.CreateOptions{})
if err != nil {
return errors.Wrapf(err, "failed to recreate CSIDriver object for driver %q", csiDriver.Name)
return errors.Wrapf(err, "failed to recreate CSIDriver object for driver %q", d.csiDriver.Name)
}
logger.Infof("CSIDriver object recreated for driver %q", csiDriver.Name)
logger.Infof("CSIDriver object recreated for driver %q", d.csiDriver.Name)
return nil
}

Expand Down
16 changes: 8 additions & 8 deletions pkg/operator/ceph/csi/csidriver.go
View file
Open in desktop
Expand Up @@ -72,6 +72,8 @@ func (d v1CsiDriver) createCSIDriverInfo(ctx context.Context, clientset kubernet
// As FSGroupPolicy field is immutable, should be set only during create time.
// if the request is to change the FSGroupPolicy, we are deleting the CSIDriver object and creating it.
if driver.Spec.FSGroupPolicy != nil && csiDriver.Spec.FSGroupPolicy != nil && *driver.Spec.FSGroupPolicy != *csiDriver.Spec.FSGroupPolicy {
d.csiClient = csidrivers
d.csiDriver = csiDriver
return d.reCreateCSIDriverInfo(ctx)
}

Expand All @@ -88,18 +90,16 @@ func (d v1CsiDriver) createCSIDriverInfo(ctx context.Context, clientset kubernet
}

func (d v1CsiDriver) reCreateCSIDriverInfo(ctx context.Context) error {
csiDriver := d.csiDriver
csiClient := d.csiClient
err := csiClient.Delete(ctx, csiDriver.Name, metav1.DeleteOptions{})
err := d.csiClient.Delete(ctx, d.csiDriver.Name, metav1.DeleteOptions{})
if err != nil {
return errors.Wrapf(err, "failed to delete CSIDriver object for driver %q", csiDriver.Name)
return errors.Wrapf(err, "failed to delete CSIDriver object for driver %q", d.csiDriver.Name)
}
logger.Infof("CSIDriver object deleted for driver %q", csiDriver.Name)
_, err = csiClient.Create(ctx, d.csiDriver, metav1.CreateOptions{})
Madhu-1 marked this conversation as resolved.
Show resolved Hide resolved
logger.Infof("CSIDriver object deleted for driver %q", d.csiDriver.Name)
_, err = d.csiClient.Create(ctx, d.csiDriver, metav1.CreateOptions{})
if err != nil {
return errors.Wrapf(err, "failed to recreate CSIDriver object for driver %q", csiDriver.Name)
return errors.Wrapf(err, "failed to recreate CSIDriver object for driver %q", d.csiDriver.Name)
}
logger.Infof("CSIDriver object recreated for driver %q", csiDriver.Name)
logger.Infof("CSIDriver object recreated for driver %q", d.csiDriver.Name)
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/operator/ceph/csi/spec.go
View file
Open in desktop
Expand Up @@ -435,7 +435,6 @@ func startDrivers(clientset kubernetes.Interface, rookclientset rookclient.Inter
return errors.Wrap(err, "failed to load rbd plugin service template")
}
rbdService.Namespace = namespace
logger.Info("successfully started CSI Ceph RBD")
}
if EnableCephFS {
cephfsPlugin, err = templateToDaemonSet("cephfsplugin", CephFSPluginTemplatePath, tp)
Expand All @@ -453,7 +452,6 @@ func startDrivers(clientset kubernetes.Interface, rookclientset rookclient.Inter
return errors.Wrap(err, "failed to load cephfs plugin service template")
}
cephfsService.Namespace = namespace
logger.Info("successfully started CSI CephFS driver")
}

// get common provisioner tolerations and node affinity
Expand Down Expand Up @@ -516,6 +514,7 @@ func startDrivers(clientset kubernetes.Interface, rookclientset rookclient.Inter
return errors.Wrapf(err, "failed to start rbd provisioner deployment: %+v", rbdProvisionerDeployment)
}
k8sutil.AddRookVersionLabelToDeployment(rbdProvisionerDeployment)
logger.Info("successfully started CSI Ceph RBD driver")
Copy link
Member

Choose a reason for hiding this comment

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

Same suggestion as below

Suggested change
logger.Info("successfully started CSI Ceph RBD driver")
logger.Info("successfully started CSI Ceph RBD provisioner")

}

if rbdService != nil {
Expand Down Expand Up @@ -584,6 +583,7 @@ func startDrivers(clientset kubernetes.Interface, rookclientset rookclient.Inter
return errors.Wrapf(err, "failed to start cephfs provisioner deployment: %+v", cephfsProvisionerDeployment)
}
k8sutil.AddRookVersionLabelToDeployment(cephfsProvisionerDeployment)
logger.Info("successfully started CSI CephFS driver")
Copy link
Member

Choose a reason for hiding this comment

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

Shall we have a separate message for the volume plugin (line 555) and the provisioner here?

Suggested change
logger.Info("successfully started CSI CephFS driver")
logger.Info("successfully started CSI CephFS provisioner")

Copy link
Member Author

Choose a reason for hiding this comment

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

@travisn as we are starting both provisioner and daemonset I left the logging as it was previously

}
if cephfsService != nil {
err = ownerInfo.SetControllerReference(cephfsService)
Expand Down