Skip to content

Update if AlreadyExists #3202

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

Merged
merged 1 commit into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/api/wrappers/deployment_install_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions pkg/controller/bundle/bundle_unpacker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

bundle_unpacker.go does not have the wrapper client; I made the changes this way instead of spending the time to plumb the client in to keep the PR simpler and spend time on more important changes.

role, err = c.client.RbacV1().Roles(fresh.GetNamespace()).Update(context.TODO(), fresh, metav1.UpdateOptions{})
}
}

return
Expand Down Expand Up @@ -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
Expand Down
23 changes: 5 additions & 18 deletions pkg/controller/operators/olm/operatorgroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}

Expand Down
10 changes: 8 additions & 2 deletions pkg/controller/operators/operatorcondition_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As in bundler_unpacker.go, client wrapper is not available here.

if apierrors.IsAlreadyExists(err) {
return r.Client.Update(context.TODO(), role)
}
}

if ownerutil.IsOwnedBy(existingRole, operatorCondition) &&
Expand Down Expand Up @@ -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) &&
Expand Down
9 changes: 7 additions & 2 deletions pkg/lib/operatorclient/apiservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 7 additions & 2 deletions pkg/lib/operatorclient/clusterrole.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 7 additions & 2 deletions pkg/lib/operatorclient/clusterrolebinding.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion pkg/lib/operatorclient/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,19 @@ 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"
)

// 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.
Expand Down
9 changes: 7 additions & 2 deletions pkg/lib/operatorclient/customresources.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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
Comment on lines +112 to 115
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, This could be:

return err == nil, err

For most, if not all, of the returns.

Expand Down
9 changes: 7 additions & 2 deletions pkg/lib/operatorclient/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Three returns? Compared to the others that just have two? (Just whining)

return updatedDep, err
}
return createdDep, err
}

// DeleteDeployment deletes the Deployment object.
Expand Down
9 changes: 7 additions & 2 deletions pkg/lib/operatorclient/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 7 additions & 2 deletions pkg/lib/operatorclient/rolebinding.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down
9 changes: 7 additions & 2 deletions pkg/lib/operatorclient/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 7 additions & 2 deletions pkg/lib/operatorclient/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down
Loading