From 9b1be42d3c6e0540016062601d1ed34e01c6e3a4 Mon Sep 17 00:00:00 2001 From: Daniel Franz Date: Fri, 12 Apr 2024 09:11:35 -0700 Subject: [PATCH] Update if AlreadyExists Adds code to the client calls for Create to run an Update if the object already exists. This allows us to bypass issues where the object was not found in cache but already exists in the cluster. Signed-off-by: Daniel Franz --- pkg/api/wrappers/deployment_install_client.go | 4 ++-- pkg/controller/bundle/bundle_unpacker.go | 6 +++++ pkg/controller/operators/olm/operatorgroup.go | 23 ++++--------------- .../operators/operatorcondition_controller.go | 10 ++++++-- pkg/lib/operatorclient/apiservice.go | 9 ++++++-- pkg/lib/operatorclient/clusterrole.go | 9 ++++++-- pkg/lib/operatorclient/clusterrolebinding.go | 9 ++++++-- pkg/lib/operatorclient/configmap.go | 7 +++++- pkg/lib/operatorclient/customresources.go | 9 ++++++-- pkg/lib/operatorclient/deployment.go | 9 ++++++-- pkg/lib/operatorclient/role.go | 9 ++++++-- pkg/lib/operatorclient/rolebinding.go | 9 ++++++-- pkg/lib/operatorclient/secret.go | 9 ++++++-- pkg/lib/operatorclient/service.go | 9 ++++++-- 14 files changed, 90 insertions(+), 41 deletions(-) diff --git a/pkg/api/wrappers/deployment_install_client.go b/pkg/api/wrappers/deployment_install_client.go index dc3fcd32e0..f2bc748c22 100644 --- a/pkg/api/wrappers/deployment_install_client.go +++ b/pkg/api/wrappers/deployment_install_client.go @@ -58,11 +58,11 @@ func (c *InstallStrategyDeploymentClientForNamespace) GetOpLister() operatorlist } func (c *InstallStrategyDeploymentClientForNamespace) CreateRole(role *rbacv1.Role) (*rbacv1.Role, error) { - return c.opClient.KubernetesInterface().RbacV1().Roles(c.Namespace).Create(context.TODO(), role, metav1.CreateOptions{}) + return c.opClient.CreateRole(role) } func (c *InstallStrategyDeploymentClientForNamespace) CreateRoleBinding(roleBinding *rbacv1.RoleBinding) (*rbacv1.RoleBinding, error) { - return c.opClient.KubernetesInterface().RbacV1().RoleBindings(c.Namespace).Create(context.TODO(), roleBinding, metav1.CreateOptions{}) + return c.opClient.CreateRoleBinding(roleBinding) } func (c *InstallStrategyDeploymentClientForNamespace) EnsureServiceAccount(serviceAccount *corev1.ServiceAccount, owner ownerutil.Owner) (*corev1.ServiceAccount, error) { diff --git a/pkg/controller/bundle/bundle_unpacker.go b/pkg/controller/bundle/bundle_unpacker.go index 531c6bdde6..4836706d20 100644 --- a/pkg/controller/bundle/bundle_unpacker.go +++ b/pkg/controller/bundle/bundle_unpacker.go @@ -734,6 +734,9 @@ func (c *ConfigMapUnpacker) ensureRole(cmRef *corev1.ObjectReference) (role *rba if err != nil { if apierrors.IsNotFound(err) { role, err = c.client.RbacV1().Roles(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + role, err = c.client.RbacV1().Roles(fresh.GetNamespace()).Update(context.TODO(), fresh, metav1.UpdateOptions{}) + } } return @@ -778,6 +781,9 @@ func (c *ConfigMapUnpacker) ensureRoleBinding(cmRef *corev1.ObjectReference) (ro if err != nil { if apierrors.IsNotFound(err) { roleBinding, err = c.client.RbacV1().RoleBindings(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + roleBinding, err = c.client.RbacV1().RoleBindings(fresh.GetNamespace()).Update(context.TODO(), fresh, metav1.UpdateOptions{}) + } } return diff --git a/pkg/controller/operators/olm/operatorgroup.go b/pkg/controller/operators/olm/operatorgroup.go index 3594c544f4..73a52caa9d 100644 --- a/pkg/controller/operators/olm/operatorgroup.go +++ b/pkg/controller/operators/olm/operatorgroup.go @@ -408,11 +408,8 @@ func (a *Operator) ensureProvidedAPIClusterRole(namePrefix, suffix string, verbs return err } if apierrors.IsNotFound(err) { - existingCR, err = a.opClient.KubernetesInterface().RbacV1().ClusterRoles().Create(context.TODO(), clusterRole, metav1.CreateOptions{}) - if err == nil { - return nil - } - if !apierrors.IsAlreadyExists(err) { + existingCR, err = a.opClient.CreateClusterRole(clusterRole) + if err != nil { a.logger.WithError(err).Errorf("Create cluster role failed: %v", clusterRole) return err } @@ -546,12 +543,7 @@ func (a *Operator) ensureSingletonRBAC(operatorNamespace string, csv *v1alpha1.C Resources: []string{"namespaces"}, }), } - // TODO: this should do something smarter if the cluster role already exists - if cr, err := a.opClient.CreateClusterRole(clusterRole); err != nil { - // If the CR already exists, but the label is correct, the cache is just behind - if apierrors.IsAlreadyExists(err) && cr != nil && ownerutil.IsOwnedByLabel(cr, csv) { - continue - } + if _, err := a.opClient.CreateClusterRole(clusterRole); err != nil { return err } a.logger.Debug("created cluster role") @@ -585,12 +577,7 @@ func (a *Operator) ensureSingletonRBAC(operatorNamespace string, csv *v1alpha1.C Name: r.RoleRef.Name, }, } - // TODO: this should do something smarter if the cluster role binding already exists - if crb, err := a.opClient.CreateClusterRoleBinding(clusterRoleBinding); err != nil { - // If the CRB already exists, but the label is correct, the cache is just behind - if apierrors.IsAlreadyExists(err) && crb != nil && ownerutil.IsOwnedByLabel(crb, csv) { - continue - } + if _, err := a.opClient.CreateClusterRoleBinding(clusterRoleBinding); err != nil { return err } } @@ -1056,7 +1043,7 @@ func (a *Operator) ensureOpGroupClusterRole(op *operatorsv1.OperatorGroup, suffi clusterRole.Labels[install.OLMManagedLabelKey] = install.OLMManagedLabelValue a.logger.Infof("creating cluster role: %s owned by operator group: %s/%s", clusterRole.GetName(), op.GetNamespace(), op.GetName()) - _, err = a.opClient.KubernetesInterface().RbacV1().ClusterRoles().Create(context.TODO(), clusterRole, metav1.CreateOptions{}) + _, err = a.opClient.CreateClusterRole(clusterRole) return err } diff --git a/pkg/controller/operators/operatorcondition_controller.go b/pkg/controller/operators/operatorcondition_controller.go index c805977c01..ab462fee37 100644 --- a/pkg/controller/operators/operatorcondition_controller.go +++ b/pkg/controller/operators/operatorcondition_controller.go @@ -150,7 +150,10 @@ func (r *OperatorConditionReconciler) ensureOperatorConditionRole(operatorCondit if !apierrors.IsNotFound(err) { return err } - return r.Client.Create(context.TODO(), role) + err = r.Client.Create(context.TODO(), role) + if apierrors.IsAlreadyExists(err) { + return r.Client.Update(context.TODO(), role) + } } if ownerutil.IsOwnedBy(existingRole, operatorCondition) && @@ -199,7 +202,10 @@ func (r *OperatorConditionReconciler) ensureOperatorConditionRoleBinding(operato if !apierrors.IsNotFound(err) { return err } - return r.Client.Create(context.TODO(), roleBinding) + err = r.Client.Create(context.TODO(), roleBinding) + if apierrors.IsAlreadyExists(err) { + return r.Client.Update(context.TODO(), roleBinding) + } } if ownerutil.IsOwnedBy(existingRoleBinding, operatorCondition) && diff --git a/pkg/lib/operatorclient/apiservice.go b/pkg/lib/operatorclient/apiservice.go index 2267a85ad8..ceea0e18a7 100644 --- a/pkg/lib/operatorclient/apiservice.go +++ b/pkg/lib/operatorclient/apiservice.go @@ -4,15 +4,20 @@ import ( "context" "fmt" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/klog" apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" ) -// CreateAPIService creates the APIService. +// CreateAPIService creates the APIService or Updates if it already exists. func (c *Client) CreateAPIService(ig *apiregistrationv1.APIService) (*apiregistrationv1.APIService, error) { - return c.ApiregistrationV1Interface().ApiregistrationV1().APIServices().Create(context.TODO(), ig, metav1.CreateOptions{}) + createdAS, err := c.ApiregistrationV1Interface().ApiregistrationV1().APIServices().Create(context.TODO(), ig, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + return c.UpdateAPIService(ig) + } + return createdAS, err } // GetAPIService returns the existing APIService. diff --git a/pkg/lib/operatorclient/clusterrole.go b/pkg/lib/operatorclient/clusterrole.go index 58db0fc795..e9060994d5 100644 --- a/pkg/lib/operatorclient/clusterrole.go +++ b/pkg/lib/operatorclient/clusterrole.go @@ -5,14 +5,19 @@ import ( "fmt" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/klog" ) -// CreateClusterRole creates the ClusterRole. +// CreateClusterRole creates the ClusterRole or Updates if it already exists. func (c *Client) CreateClusterRole(r *rbacv1.ClusterRole) (*rbacv1.ClusterRole, error) { - return c.RbacV1().ClusterRoles().Create(context.TODO(), r, metav1.CreateOptions{}) + createdClusterRole, err := c.RbacV1().ClusterRoles().Create(context.TODO(), r, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + return c.UpdateClusterRole(r) + } + return createdClusterRole, err } // GetClusterRole returns the existing ClusterRole. diff --git a/pkg/lib/operatorclient/clusterrolebinding.go b/pkg/lib/operatorclient/clusterrolebinding.go index 0ab429accc..24cc3177c9 100644 --- a/pkg/lib/operatorclient/clusterrolebinding.go +++ b/pkg/lib/operatorclient/clusterrolebinding.go @@ -5,6 +5,7 @@ import ( "fmt" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" acv1 "k8s.io/client-go/applyconfigurations/rbac/v1" @@ -16,9 +17,13 @@ func (c *Client) ApplyClusterRoleBinding(applyConfig *acv1.ClusterRoleBindingApp return c.RbacV1().ClusterRoleBindings().Apply(context.TODO(), applyConfig, applyOptions) } -// CreateRoleBinding creates the roleBinding. +// CreateRoleBinding creates the roleBinding or Updates if it already exists. func (c *Client) CreateClusterRoleBinding(ig *rbacv1.ClusterRoleBinding) (*rbacv1.ClusterRoleBinding, error) { - return c.RbacV1().ClusterRoleBindings().Create(context.TODO(), ig, metav1.CreateOptions{}) + createdCRB, err := c.RbacV1().ClusterRoleBindings().Create(context.TODO(), ig, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + return c.UpdateClusterRoleBinding(ig) + } + return createdCRB, err } // GetRoleBinding returns the existing roleBinding. diff --git a/pkg/lib/operatorclient/configmap.go b/pkg/lib/operatorclient/configmap.go index 0ac2e7c013..44565fe6a4 100644 --- a/pkg/lib/operatorclient/configmap.go +++ b/pkg/lib/operatorclient/configmap.go @@ -5,6 +5,7 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/klog" @@ -12,7 +13,11 @@ import ( // CreateConfigMap creates the ConfigMap. func (c *Client) CreateConfigMap(ig *corev1.ConfigMap) (*corev1.ConfigMap, error) { - return c.CoreV1().ConfigMaps(ig.GetNamespace()).Create(context.TODO(), ig, metav1.CreateOptions{}) + createdCM, err := c.CoreV1().ConfigMaps(ig.GetNamespace()).Create(context.TODO(), ig, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + return c.UpdateConfigMap(ig) + } + return createdCM, err } // GetConfigMap returns the existing ConfigMap. diff --git a/pkg/lib/operatorclient/customresources.go b/pkg/lib/operatorclient/customresources.go index 2d4b7b26a5..0ee0f0eb40 100644 --- a/pkg/lib/operatorclient/customresources.go +++ b/pkg/lib/operatorclient/customresources.go @@ -9,6 +9,7 @@ import ( "time" "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/wait" @@ -92,7 +93,7 @@ func (c *Client) CreateCustomResourceRaw(apiGroup, version, namespace, kind stri return nil } -// CreateCustomResourceRawIfNotFound creates the raw bytes of the custom resource if it doesn't exist. +// CreateCustomResourceRawIfNotFound creates the raw bytes of the custom resource if it doesn't exist, or Updates if it does exist. // It also returns a boolean to indicate whether a new custom resource is created. func (c *Client) CreateCustomResourceRawIfNotFound(apiGroup, version, namespace, kind, name string, data []byte) (bool, error) { klog.V(4).Infof("[CREATE CUSTOM RESOURCE RAW if not found]: %s:%s", namespace, name) @@ -104,7 +105,11 @@ func (c *Client) CreateCustomResourceRawIfNotFound(apiGroup, version, namespace, return false, err } err = c.CreateCustomResourceRaw(apiGroup, version, namespace, kind, data) - if err != nil { + if apierrors.IsAlreadyExists(err) { + if err = c.UpdateCustomResourceRaw(apiGroup, version, namespace, kind, name, data); err != nil { + return false, err + } + } else if err != nil { return false, err } return true, nil diff --git a/pkg/lib/operatorclient/deployment.go b/pkg/lib/operatorclient/deployment.go index 327826f875..b7e3632d0b 100644 --- a/pkg/lib/operatorclient/deployment.go +++ b/pkg/lib/operatorclient/deployment.go @@ -25,10 +25,15 @@ func (c *Client) GetDeployment(namespace, name string) (*appsv1.Deployment, erro return c.AppsV1().Deployments(namespace).Get(context.TODO(), name, metav1.GetOptions{}) } -// CreateDeployment creates the Deployment object. +// CreateDeployment creates the Deployment object or Updates if it already exists. func (c *Client) CreateDeployment(dep *appsv1.Deployment) (*appsv1.Deployment, error) { klog.V(4).Infof("[CREATE Deployment]: %s:%s", dep.Namespace, dep.Name) - return c.AppsV1().Deployments(dep.Namespace).Create(context.TODO(), dep, metav1.CreateOptions{}) + createdDep, err := c.AppsV1().Deployments(dep.Namespace).Create(context.TODO(), dep, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + updatedDep, _, err := c.UpdateDeployment(dep) + return updatedDep, err + } + return createdDep, err } // DeleteDeployment deletes the Deployment object. diff --git a/pkg/lib/operatorclient/role.go b/pkg/lib/operatorclient/role.go index b3cacd95d7..7f6700c4fc 100644 --- a/pkg/lib/operatorclient/role.go +++ b/pkg/lib/operatorclient/role.go @@ -5,14 +5,19 @@ import ( "fmt" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/klog" ) -// CreateRole creates the role. +// CreateRole creates the role or Updates if it already exists. func (c *Client) CreateRole(r *rbacv1.Role) (*rbacv1.Role, error) { - return c.RbacV1().Roles(r.GetNamespace()).Create(context.TODO(), r, metav1.CreateOptions{}) + createdRole, err := c.RbacV1().Roles(r.GetNamespace()).Create(context.TODO(), r, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + return c.UpdateRole(r) + } + return createdRole, err } // GetRole returns the existing role. diff --git a/pkg/lib/operatorclient/rolebinding.go b/pkg/lib/operatorclient/rolebinding.go index 44fc637c1c..8fe41cfe0d 100644 --- a/pkg/lib/operatorclient/rolebinding.go +++ b/pkg/lib/operatorclient/rolebinding.go @@ -5,6 +5,7 @@ import ( "fmt" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" acv1 "k8s.io/client-go/applyconfigurations/rbac/v1" @@ -16,9 +17,13 @@ func (c *Client) ApplyRoleBinding(applyConfig *acv1.RoleBindingApplyConfiguratio return c.RbacV1().RoleBindings(*applyConfig.Namespace).Apply(context.TODO(), applyConfig, applyOptions) } -// CreateRoleBinding creates the roleBinding. +// CreateRoleBinding creates the roleBinding or Updates if it already exists. func (c *Client) CreateRoleBinding(ig *rbacv1.RoleBinding) (*rbacv1.RoleBinding, error) { - return c.RbacV1().RoleBindings(ig.GetNamespace()).Create(context.TODO(), ig, metav1.CreateOptions{}) + createdRB, err := c.RbacV1().RoleBindings(ig.GetNamespace()).Create(context.TODO(), ig, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + return c.UpdateRoleBinding(ig) + } + return createdRB, err } // GetRoleBinding returns the existing roleBinding. diff --git a/pkg/lib/operatorclient/secret.go b/pkg/lib/operatorclient/secret.go index 5727a27249..6f74e51035 100644 --- a/pkg/lib/operatorclient/secret.go +++ b/pkg/lib/operatorclient/secret.go @@ -5,14 +5,19 @@ import ( "fmt" v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/klog" ) -// CreateSecret creates the Secret. +// CreateSecret creates the Secret or Updates if it already exists. func (c *Client) CreateSecret(ig *v1.Secret) (*v1.Secret, error) { - return c.CoreV1().Secrets(ig.GetNamespace()).Create(context.TODO(), ig, metav1.CreateOptions{}) + createdSecret, err := c.CoreV1().Secrets(ig.GetNamespace()).Create(context.TODO(), ig, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + return c.UpdateSecret(ig) + } + return createdSecret, err } // GetSecret returns the existing Secret. diff --git a/pkg/lib/operatorclient/service.go b/pkg/lib/operatorclient/service.go index 89c593e67e..63c0800954 100644 --- a/pkg/lib/operatorclient/service.go +++ b/pkg/lib/operatorclient/service.go @@ -5,6 +5,7 @@ import ( "fmt" v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" acv1 "k8s.io/client-go/applyconfigurations/core/v1" @@ -16,9 +17,13 @@ func (c *Client) ApplyService(applyConfig *acv1.ServiceApplyConfiguration, apply return c.CoreV1().Services(*applyConfig.Namespace).Apply(context.TODO(), applyConfig, applyOptions) } -// CreateService creates the Service. +// CreateService creates the Service or Updates if it already exists. func (c *Client) CreateService(ig *v1.Service) (*v1.Service, error) { - return c.CoreV1().Services(ig.GetNamespace()).Create(context.TODO(), ig, metav1.CreateOptions{}) + createdService, err := c.CoreV1().Services(ig.GetNamespace()).Create(context.TODO(), ig, metav1.CreateOptions{}) + if apierrors.IsAlreadyExists(err) { + return c.UpdateService(ig) + } + return createdService, err } // GetService returns the existing Service.