From 7544b53693f509812a707f04e824961068f80133 Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Wed, 10 Apr 2019 18:10:36 +0530 Subject: [PATCH 1/5] Create endpoint/service early to avoid unwanted create/delete volume transaction. At times, for some reason endpoint/service creation can fail in a setup. As we currently create endpoint/service after volume creation, later we need rollback of this volume transaction if endpoint/service creation failed. Considering endpoint/service creation is light weight, this patch promote endpoint/service creation to an early stage. Signed-off-by: Humble Chirammal --- pkg/volume/glusterfs/glusterfs.go | 60 +++++++++++-------- .../rbac/bootstrappolicy/controller_policy.go | 3 +- .../testdata/controller-roles.yaml | 8 +++ 3 files changed, 46 insertions(+), 25 deletions(-) diff --git a/pkg/volume/glusterfs/glusterfs.go b/pkg/volume/glusterfs/glusterfs.go index 6b0b5ab3421..a58208fd5f4 100644 --- a/pkg/volume/glusterfs/glusterfs.go +++ b/pkg/volume/glusterfs/glusterfs.go @@ -809,6 +809,19 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi customVolumeName := "" epServiceName := "" + if len(p.provisionerConfig.customEpNamePrefix) == 0 { + epServiceName = string(p.options.PVC.UID) + } else { + epServiceName = p.provisionerConfig.customEpNamePrefix + "-" + string(p.options.PVC.UID) + } + epNamespace := p.options.PVC.Namespace + endpoint, service, err := p.createEndpointService(epNamespace, epServiceName, p.options.PVC.Name) + if err != nil { + klog.Errorf("failed to create endpoint/service %v/%v: %v", epNamespace, epServiceName, err) + return nil, 0, "", fmt.Errorf("failed to create endpoint/service %v/%v: %v", epNamespace, epServiceName, err) + } + klog.V(3).Infof("dynamic endpoint %v and service %v ", endpoint, service) + capacity := p.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] // GlusterFS/heketi creates volumes in units of GiB. @@ -859,22 +872,28 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi return nil, 0, "", fmt.Errorf("failed to get cluster nodes for volume %s: %v", volume, err) } - if len(p.provisionerConfig.customEpNamePrefix) == 0 { - epServiceName = string(p.options.PVC.UID) - } else { - epServiceName = p.provisionerConfig.customEpNamePrefix + "-" + string(p.options.PVC.UID) + addrlist := make([]v1.EndpointAddress, len(dynamicHostIps)) + for i, v := range dynamicHostIps { + addrlist[i].IP = v } - epNamespace := p.options.PVC.Namespace - endpoint, service, err := p.createEndpointService(epNamespace, epServiceName, dynamicHostIps, p.options.PVC) + subset := make([]v1.EndpointSubset, 1) + ports := []v1.EndpointPort{{Port: 1, Protocol: "TCP"}} + + endpoint.Subsets = subset + endpoint.Subsets[0].Addresses = addrlist + endpoint.Subsets[0].Ports = ports + + kubeClient := p.plugin.host.GetKubeClient() + if kubeClient == nil { + return nil, 0, "", fmt.Errorf("failed to get kube client to update endpoint") + } + _, err = kubeClient.CoreV1().Endpoints(epNamespace).Update(endpoint) if err != nil { - klog.Errorf("failed to create endpoint/service %v/%v: %v", epNamespace, epServiceName, err) - deleteErr := cli.VolumeDelete(volume.Id) - if deleteErr != nil { - klog.Errorf("failed to delete volume: %v, manual deletion of the volume required", deleteErr) - } - return nil, 0, "", fmt.Errorf("failed to create endpoint/service %v/%v: %v", epNamespace, epServiceName, err) + return nil, 0, "", fmt.Errorf("failed to update endpoint %s: %v", endpoint, err) } - klog.V(3).Infof("dynamic endpoint %v and service %v ", endpoint, service) + + klog.V(3).Infof("endpoint %s updated successfully", endpoint) + return &v1.GlusterfsPersistentVolumeSource{ EndpointsName: endpoint.Name, EndpointsNamespace: &epNamespace, @@ -884,10 +903,11 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi } // createEndpointService() makes sure an endpoint and service -// exist for the given namespace, PVC name, endpoint name, and -// set of IPs. I.e. the endpoint or service is only created +// exist for the given namespace, PVC name, endpoint name +// I.e. the endpoint or service is only created // if it does not exist yet. -func (p *glusterfsVolumeProvisioner) createEndpointService(namespace string, epServiceName string, hostips []string, pvc *v1.PersistentVolumeClaim) (endpoint *v1.Endpoints, service *v1.Service, err error) { + +func (p *glusterfsVolumeProvisioner) createEndpointService(namespace string, epServiceName string, pvcname string) (endpoint *v1.Endpoints, service *v1.Service, err error) { pvcNameOrID := "" if len(pvc.Name) >= 63 { pvcNameOrID = string(pvc.UID) @@ -895,10 +915,6 @@ func (p *glusterfsVolumeProvisioner) createEndpointService(namespace string, epS pvcNameOrID = pvc.Name } - addrlist := make([]v1.EndpointAddress, len(hostips)) - for i, v := range hostips { - addrlist[i].IP = v - } endpoint = &v1.Endpoints{ ObjectMeta: metav1.ObjectMeta{ Namespace: namespace, @@ -907,10 +923,6 @@ func (p *glusterfsVolumeProvisioner) createEndpointService(namespace string, epS "gluster.kubernetes.io/provisioned-for-pvc": pvcNameOrID, }, }, - Subsets: []v1.EndpointSubset{{ - Addresses: addrlist, - Ports: []v1.EndpointPort{{Port: 1, Protocol: "TCP"}}, - }}, } kubeClient := p.plugin.host.GetKubeClient() if kubeClient == nil { diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go index d1e83dd99cd..10b1e52ad78 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go @@ -230,7 +230,8 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding) // glusterfs rbacv1helpers.NewRule("get", "list", "watch").Groups(storageGroup).Resources("storageclasses").RuleOrDie(), - rbacv1helpers.NewRule("get", "create", "delete").Groups(legacyGroup).Resources("services", "endpoints").RuleOrDie(), + rbacv1helpers.NewRule("get", "create", "update", "delete").Groups(legacyGroup).Resources("endpoints").RuleOrDie(), + rbacv1helpers.NewRule("get", "create", "delete").Groups(legacyGroup).Resources("services").RuleOrDie(), rbacv1helpers.NewRule("get").Groups(legacyGroup).Resources("secrets").RuleOrDie(), // openstack rbacv1helpers.NewRule("get", "list").Groups(legacyGroup).Resources("nodes").RuleOrDie(), diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml index 2c64f63a963..572ccfae033 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml @@ -789,6 +789,14 @@ items: - "" resources: - endpoints + verbs: + - create + - delete + - get + - update + - apiGroups: + - "" + resources: - services verbs: - create From 58e65c053a349c0ef27f6bf1f729a1d5f77fc56f Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Wed, 10 Apr 2019 20:48:44 +0530 Subject: [PATCH 2/5] Resolve merge conflict Signed-off-by: Humble Chirammal --- pkg/volume/glusterfs/glusterfs.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/volume/glusterfs/glusterfs.go b/pkg/volume/glusterfs/glusterfs.go index a58208fd5f4..7ca8d02a726 100644 --- a/pkg/volume/glusterfs/glusterfs.go +++ b/pkg/volume/glusterfs/glusterfs.go @@ -815,7 +815,7 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi epServiceName = p.provisionerConfig.customEpNamePrefix + "-" + string(p.options.PVC.UID) } epNamespace := p.options.PVC.Namespace - endpoint, service, err := p.createEndpointService(epNamespace, epServiceName, p.options.PVC.Name) + endpoint, service, err := p.createEndpointService(epNamespace, epServiceName, p.options.PVC) if err != nil { klog.Errorf("failed to create endpoint/service %v/%v: %v", epNamespace, epServiceName, err) return nil, 0, "", fmt.Errorf("failed to create endpoint/service %v/%v: %v", epNamespace, epServiceName, err) @@ -906,8 +906,7 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi // exist for the given namespace, PVC name, endpoint name // I.e. the endpoint or service is only created // if it does not exist yet. - -func (p *glusterfsVolumeProvisioner) createEndpointService(namespace string, epServiceName string, pvcname string) (endpoint *v1.Endpoints, service *v1.Service, err error) { +func (p *glusterfsVolumeProvisioner) createEndpointService(namespace string, epServiceName string, pvc *v1.PersistentVolumeClaim) (endpoint *v1.Endpoints, service *v1.Service, err error) { pvcNameOrID := "" if len(pvc.Name) >= 63 { pvcNameOrID = string(pvc.UID) From c86828b74b39a4cd36d1cd70155a9991e8bcf55e Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Tue, 16 Apr 2019 11:50:42 +0530 Subject: [PATCH 3/5] Cleanup volume, ep/svc if endpoint update failed. Signed-off-by: Humble Chirammal --- pkg/volume/glusterfs/glusterfs.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/volume/glusterfs/glusterfs.go b/pkg/volume/glusterfs/glusterfs.go index 7ca8d02a726..ff597b9d3c2 100644 --- a/pkg/volume/glusterfs/glusterfs.go +++ b/pkg/volume/glusterfs/glusterfs.go @@ -889,7 +889,20 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi } _, err = kubeClient.CoreV1().Endpoints(epNamespace).Update(endpoint) if err != nil { + deleteErr := cli.VolumeDelete(volume.Id) + if deleteErr != nil { + klog.Errorf("failed to delete volume: %v, manual deletion of the volume required", deleteErr) + } + + klog.V(3).Infof("failed to update endpoint, deleting %s", endpoint) + + err = kubeClient.CoreV1().Services(epNamespace).Delete(epServiceName, nil) + if err != nil { + klog.Errorf("failed to delete service %s/%s: %v", epNamespace, epServiceName, err) + } + klog.V(1).Infof("service/endpoint: %s/%s deleted successfully", epNamespace, epServiceName) return nil, 0, "", fmt.Errorf("failed to update endpoint %s: %v", endpoint, err) + } klog.V(3).Infof("endpoint %s updated successfully", endpoint) From 745f12837f849cd9854c451fb2b3fcc807bc2d1e Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Thu, 2 May 2019 21:06:34 +0530 Subject: [PATCH 4/5] Rename createEndpointService() to createOrGetEndpointService() Signed-off-by: Humble Chirammal --- pkg/volume/glusterfs/glusterfs.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/volume/glusterfs/glusterfs.go b/pkg/volume/glusterfs/glusterfs.go index ff597b9d3c2..30ff3d64330 100644 --- a/pkg/volume/glusterfs/glusterfs.go +++ b/pkg/volume/glusterfs/glusterfs.go @@ -815,7 +815,7 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi epServiceName = p.provisionerConfig.customEpNamePrefix + "-" + string(p.options.PVC.UID) } epNamespace := p.options.PVC.Namespace - endpoint, service, err := p.createEndpointService(epNamespace, epServiceName, p.options.PVC) + endpoint, service, err := p.createOrGetEndpointService(epNamespace, epServiceName, p.options.PVC) if err != nil { klog.Errorf("failed to create endpoint/service %v/%v: %v", epNamespace, epServiceName, err) return nil, 0, "", fmt.Errorf("failed to create endpoint/service %v/%v: %v", epNamespace, epServiceName, err) @@ -915,11 +915,11 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi }, sz, volID, nil } -// createEndpointService() makes sure an endpoint and service +// createOrGetEndpointService() makes sure an endpoint and service // exist for the given namespace, PVC name, endpoint name // I.e. the endpoint or service is only created // if it does not exist yet. -func (p *glusterfsVolumeProvisioner) createEndpointService(namespace string, epServiceName string, pvc *v1.PersistentVolumeClaim) (endpoint *v1.Endpoints, service *v1.Service, err error) { +func (p *glusterfsVolumeProvisioner) createOrGetEndpointService(namespace string, epServiceName string, pvc *v1.PersistentVolumeClaim) (endpoint *v1.Endpoints, service *v1.Service, err error) { pvcNameOrID := "" if len(pvc.Name) >= 63 { pvcNameOrID = string(pvc.UID) From e47a7897ea608c85715cc647b777d0dc276eafbf Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Tue, 14 May 2019 10:29:59 +0530 Subject: [PATCH 5/5] Fail early if driver cant get kubeclient Signed-off-by: Humble Chirammal --- pkg/volume/glusterfs/glusterfs.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/volume/glusterfs/glusterfs.go b/pkg/volume/glusterfs/glusterfs.go index 30ff3d64330..e7031625fd1 100644 --- a/pkg/volume/glusterfs/glusterfs.go +++ b/pkg/volume/glusterfs/glusterfs.go @@ -809,6 +809,11 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi customVolumeName := "" epServiceName := "" + kubeClient := p.plugin.host.GetKubeClient() + if kubeClient == nil { + return nil, 0, "", fmt.Errorf("failed to get kube client to update endpoint") + } + if len(p.provisionerConfig.customEpNamePrefix) == 0 { epServiceName = string(p.options.PVC.UID) } else { @@ -883,10 +888,6 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi endpoint.Subsets[0].Addresses = addrlist endpoint.Subsets[0].Ports = ports - kubeClient := p.plugin.host.GetKubeClient() - if kubeClient == nil { - return nil, 0, "", fmt.Errorf("failed to get kube client to update endpoint") - } _, err = kubeClient.CoreV1().Endpoints(epNamespace).Update(endpoint) if err != nil { deleteErr := cli.VolumeDelete(volume.Id) @@ -897,6 +898,11 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi klog.V(3).Infof("failed to update endpoint, deleting %s", endpoint) err = kubeClient.CoreV1().Services(epNamespace).Delete(epServiceName, nil) + + if err != nil && errors.IsNotFound(err) { + klog.V(1).Infof("service %s does not exist in namespace %s", epServiceName, epNamespace) + err = nil + } if err != nil { klog.Errorf("failed to delete service %s/%s: %v", epNamespace, epServiceName, err) }