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.

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

fix: Use cached and non-cached clients in backup/restore controllers #1156

Merged
merged 1 commit into from
Oct 21, 2021
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
2 changes: 1 addition & 1 deletion controllers/checlusterbackup/backup_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func NewBackupContext(r *ReconcileCheClusterBackup, backupCR *chev1.CheClusterBa
// After the preparations, a new reconcile loop will be triggered, so backupServer will not be nil any more.
}

cheCR, _, err := util.FindCheClusterCRInNamespace(r.client, namespace)
cheCR, _, err := util.FindCheClusterCRInNamespace(r.cachingClient, namespace)
if err != nil {
return nil, err
}
Expand Down
8 changes: 4 additions & 4 deletions controllers/checlusterbackup/backup_data_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func createBackupMetadataFile(bctx *BackupContext, destDir string) (bool, error)

var appsDomain string
if util.IsOpenShift {
host, err := util.GetRouterCanonicalHostname(bctx.r.client, bctx.namespace)
host, err := util.GetRouterCanonicalHostname(bctx.r.nonCachingClient, bctx.namespace)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -129,7 +129,7 @@ func prepareDirectory(destDir string) error {
func backupCheCR(bctx *BackupContext, destDir string) (bool, error) {
cheCR := &orgv1.CheCluster{}
namespacedName := types.NamespacedName{Namespace: bctx.namespace, Name: bctx.cheCR.GetName()}
if err := bctx.r.client.Get(context.TODO(), namespacedName, cheCR); err != nil {
if err := bctx.r.cachingClient.Get(context.TODO(), namespacedName, cheCR); err != nil {
return false, err
}
util.ClearMetadata(&cheCR.ObjectMeta)
Expand Down Expand Up @@ -221,7 +221,7 @@ func backupConfigMaps(bctx *BackupContext, destDir string) (bool, error) {
}

fakeDeployContext := &deploy.DeployContext{
ClusterAPI: deploy.ClusterAPI{Client: bctx.r.client},
ClusterAPI: deploy.ClusterAPI{Client: bctx.r.nonCachingClient},
CheCluster: bctx.cheCR,
}
caBundlesConfigmaps, err := deploy.GetCACertsConfigMaps(fakeDeployContext)
Expand Down Expand Up @@ -268,7 +268,7 @@ func backupSecrets(bctx *BackupContext, destDir string) (bool, error) {
for _, secretName := range secretsNames {
secret := &corev1.Secret{}
namespacedName := types.NamespacedName{Name: secretName, Namespace: bctx.namespace}
if err := bctx.r.client.Get(context.TODO(), namespacedName, secret); err != nil {
if err := bctx.r.nonCachingClient.Get(context.TODO(), namespacedName, secret); err != nil {
return false, err
}

Expand Down
20 changes: 10 additions & 10 deletions controllers/checlusterbackup/checlusterbackup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/source"
)
Expand All @@ -43,15 +42,16 @@ const (
type ReconcileCheClusterBackup struct {
// This client, initialized using mgr.Client() above, is a split client
// that reads objects from the cache and writes to the apiserver
client client.Client
scheme *runtime.Scheme
cachingClient client.Client
nonCachingClient client.Client
scheme *runtime.Scheme
// the namespace to which to limit the reconciliation. If empty, all namespaces are considered
namespace string
}

// NewReconciler returns a new reconcile.Reconciler
func NewReconciler(mgr manager.Manager, namespace string) *ReconcileCheClusterBackup {
return &ReconcileCheClusterBackup{client: mgr.GetClient(), scheme: mgr.GetScheme(), namespace: namespace}
func NewReconciler(cachingClient client.Client, noncachingClient client.Client, scheme *runtime.Scheme, namespace string) *ReconcileCheClusterBackup {
return &ReconcileCheClusterBackup{cachingClient: cachingClient, nonCachingClient: noncachingClient, scheme: scheme, namespace: namespace}
}

// SetupWithManager sets up the controller with the Manager.
Expand Down Expand Up @@ -92,7 +92,7 @@ func (r *ReconcileCheClusterBackup) SetupWithManager(mgr ctrl.Manager) error {
func (r *ReconcileCheClusterBackup) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
// Fetch the CheClusterBackup instance
backupCR := &chev1.CheClusterBackup{}
err := r.client.Get(context.TODO(), request.NamespacedName, backupCR)
err := r.cachingClient.Get(context.TODO(), request.NamespacedName, backupCR)
if err != nil {
if errors.IsNotFound(err) {
// Request object not found, could have been deleted after reconcile request.
Expand Down Expand Up @@ -156,7 +156,7 @@ func (r *ReconcileCheClusterBackup) doReconcile(backupCR *chev1.CheClusterBackup
if backupCR.Spec.BackupServerConfigRef != "" {
backupServerConfigCR = &chev1.CheBackupServerConfiguration{}
backupServerConfigNamespacedName := types.NamespacedName{Namespace: backupCR.GetNamespace(), Name: backupCR.Spec.BackupServerConfigRef}
if err := r.client.Get(context.TODO(), backupServerConfigNamespacedName, backupServerConfigCR); err != nil {
if err := r.cachingClient.Get(context.TODO(), backupServerConfigNamespacedName, backupServerConfigCR); err != nil {
if errors.IsNotFound(err) {
return true, fmt.Errorf("backup server configuration with name '%s' not found in '%s' namespace", backupCR.Spec.BackupServerConfigRef, backupCR.GetNamespace())
}
Expand Down Expand Up @@ -198,7 +198,7 @@ func (r *ReconcileCheClusterBackup) doReconcile(backupCR *chev1.CheClusterBackup
bctx.UpdateBackupStatusPhase()

// Make sure, that backup server configuration in the CR is valid and cache cluster resources
done, err := bctx.backupServer.PrepareConfiguration(bctx.r.client, bctx.namespace)
done, err := bctx.backupServer.PrepareConfiguration(bctx.r.nonCachingClient, bctx.namespace)
if err != nil || !done {
return done, err
}
Expand Down Expand Up @@ -261,7 +261,7 @@ func (r *ReconcileCheClusterBackup) doReconcile(backupCR *chev1.CheClusterBackup
}

func (r *ReconcileCheClusterBackup) UpdateCR(cr *chev1.CheClusterBackup) error {
err := r.client.Update(context.TODO(), cr)
err := r.cachingClient.Update(context.TODO(), cr)
if err != nil {
logrus.Errorf("Failed to update %s CR: %s", cr.Name, err.Error())
return err
Expand All @@ -270,7 +270,7 @@ func (r *ReconcileCheClusterBackup) UpdateCR(cr *chev1.CheClusterBackup) error {
}

func (r *ReconcileCheClusterBackup) UpdateCRStatus(cr *chev1.CheClusterBackup) error {
err := r.client.Status().Update(context.TODO(), cr)
err := r.cachingClient.Status().Update(context.TODO(), cr)
if err != nil {
logrus.Errorf("Failed to update %s CR status: %s", cr.Name, err.Error())
return err
Expand Down
20 changes: 10 additions & 10 deletions controllers/checlusterbackup/internal_backup_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func ensureInternalBackupServerDeploymentExist(bctx *BackupContext) (bool, error
Namespace: bctx.namespace,
Name: BackupServerDeploymentName,
}
err := bctx.r.client.Get(context.TODO(), namespacedName, backupServerDeployment)
err := bctx.r.nonCachingClient.Get(context.TODO(), namespacedName, backupServerDeployment)
if err == nil {
return true, nil
}
Expand All @@ -78,7 +78,7 @@ func ensureInternalBackupServerDeploymentExist(bctx *BackupContext) (bool, error
// Get default configuration of the backup server deployment
backupServerDeployment = getBackupServerDeploymentSpec(bctx)
// Create backup server deployment
err = bctx.r.client.Create(context.TODO(), backupServerDeployment)
err = bctx.r.nonCachingClient.Create(context.TODO(), backupServerDeployment)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -149,7 +149,7 @@ func ensureInternalBackupServerPodReady(bctx *BackupContext) (bool, error) {
if err != nil {
return true, err
}
_, err = backupServer.PrepareConfiguration(bctx.r.client, bctx.namespace)
_, err = backupServer.PrepareConfiguration(bctx.r.nonCachingClient, bctx.namespace)
if err != nil {
return true, err
}
Expand All @@ -172,7 +172,7 @@ func ensureInternalBackupServerServiceExists(bctx *BackupContext) (bool, error)
Namespace: bctx.namespace,
Name: backupServerServiceName,
}
err := bctx.r.client.Get(context.TODO(), namespacedName, backupServerService)
err := bctx.r.nonCachingClient.Get(context.TODO(), namespacedName, backupServerService)
if err == nil {
// Backup server service already exists, do nothing
return true, nil
Expand All @@ -184,7 +184,7 @@ func ensureInternalBackupServerServiceExists(bctx *BackupContext) (bool, error)
// Backup server service doesn't exists, create it
backupServerService = getBackupServerServiceSpec(bctx)
// Create backup server service
err = bctx.r.client.Create(context.TODO(), backupServerService)
err = bctx.r.nonCachingClient.Create(context.TODO(), backupServerService)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -228,13 +228,13 @@ func ensureInternalBackupServerConfigurationExistAndCorrect(bctx *BackupContext)
// Check if the internal backup server configuration exists
internalBackupServerConfiguration := &chev1.CheBackupServerConfiguration{}
namespacedName := types.NamespacedName{Namespace: bctx.namespace, Name: BackupServerConfigurationName}
err := bctx.r.client.Get(context.TODO(), namespacedName, internalBackupServerConfiguration)
err := bctx.r.cachingClient.Get(context.TODO(), namespacedName, internalBackupServerConfiguration)
if err == nil {
// Configuration exist, check if it is correct
expectedInternalRestServerConfig := getExpectedInternalRestServerConfiguration(bctx)
if *internalBackupServerConfiguration.Spec.Rest != *expectedInternalRestServerConfig {
// Something is wrong in the configuration
if err := bctx.r.client.Delete(context.TODO(), internalBackupServerConfiguration); err != nil {
if err := bctx.r.cachingClient.Delete(context.TODO(), internalBackupServerConfiguration); err != nil {
return false, err
}
// Reconcile to recreate the configuration
Expand All @@ -249,7 +249,7 @@ func ensureInternalBackupServerConfigurationExistAndCorrect(bctx *BackupContext)

// Create internal backup server configuration
internalBackupServerConfiguration = getInternalBackupServerConfigurationSpec(bctx)
err = bctx.r.client.Create(context.TODO(), internalBackupServerConfiguration)
err = bctx.r.cachingClient.Create(context.TODO(), internalBackupServerConfiguration)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -310,7 +310,7 @@ func ensureInternalBackupServerSecretExists(bctx *BackupContext) (bool, error) {
Namespace: bctx.namespace,
Name: bctx.backupServerConfigCR.Spec.Rest.RepositoryPasswordSecretRef,
}
err := bctx.r.client.Get(context.TODO(), namespacedName, repoPasswordSecret)
err := bctx.r.nonCachingClient.Get(context.TODO(), namespacedName, repoPasswordSecret)
if err == nil {
return true, nil
}
Expand All @@ -320,7 +320,7 @@ func ensureInternalBackupServerSecretExists(bctx *BackupContext) (bool, error) {

repoPassword := util.GeneratePasswd(12)
repoPasswordSecret = getRepoPasswordSecretSpec(bctx, repoPassword)
err = bctx.r.client.Create(context.TODO(), repoPasswordSecret)
err = bctx.r.nonCachingClient.Create(context.TODO(), repoPasswordSecret)
if err != nil {
return false, err
}
Expand Down
42 changes: 21 additions & 21 deletions controllers/checlusterrestore/backup_data_restorer.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,14 @@ func cleanPreviousInstallation(rctx *RestoreContext, dataDir string) (bool, erro
}

// Delete Che CR to stop operator from dealing with current installation
actualCheCR, cheCRCount, err := util.FindCheClusterCRInNamespace(rctx.r.client, rctx.namespace)
actualCheCR, cheCRCount, err := util.FindCheClusterCRInNamespace(rctx.r.cachingClient, rctx.namespace)
if cheCRCount == -1 {
// error occurred while retreiving CheCluster CR
return false, err
} else if actualCheCR != nil {
if actualCheCR.GetObjectMeta().GetDeletionTimestamp().IsZero() {
logrus.Infof("Restore: Deleteing CheCluster custom resource in '%s' namespace", rctx.namespace)
err := rctx.r.client.Delete(context.TODO(), actualCheCR)
err := rctx.r.cachingClient.Delete(context.TODO(), actualCheCR)
if err == nil {
// Che CR is marked for deletion, but actually still exists.
// Wait for finalizers and actual resource deletion (not found expected).
Expand Down Expand Up @@ -161,45 +161,45 @@ func cleanPreviousInstallation(rctx *RestoreContext, dataDir string) (bool, erro

// Delete all Che related deployments, but keep operator (excluded by name) and internal backup server (excluded by label)
deploymentsList := &appsv1.DeploymentList{}
if err := rctx.r.client.List(context.TODO(), deploymentsList, cheResourcesListOptions); err != nil {
if err := rctx.r.nonCachingClient.List(context.TODO(), deploymentsList, cheResourcesListOptions); err != nil {
return false, err
}
for _, deployment := range deploymentsList.Items {
if strings.Contains(deployment.GetName(), cheFlavor+"-operator") {
continue
}
if err := rctx.r.client.Delete(context.TODO(), &deployment); err != nil && !errors.IsNotFound(err) {
if err := rctx.r.nonCachingClient.Delete(context.TODO(), &deployment); err != nil && !errors.IsNotFound(err) {
return false, err
}
}

// Delete all Che related secrets, but keep backup server ones (excluded by label)
if err := rctx.r.client.DeleteAllOf(context.TODO(), &corev1.Secret{}, client.InNamespace(rctx.namespace), cheResourcesMatchingLabelsSelector); err != nil {
if err := rctx.r.nonCachingClient.DeleteAllOf(context.TODO(), &corev1.Secret{}, client.InNamespace(rctx.namespace), cheResourcesMatchingLabelsSelector); err != nil {
return false, err
}

// Delete all configmaps with custom CA certificates
if err := rctx.r.client.DeleteAllOf(context.TODO(), &corev1.ConfigMap{}, client.InNamespace(rctx.namespace), cheResourcesMatchingLabelsSelector); err != nil {
if err := rctx.r.nonCachingClient.DeleteAllOf(context.TODO(), &corev1.ConfigMap{}, client.InNamespace(rctx.namespace), cheResourcesMatchingLabelsSelector); err != nil {
return false, err
}

// Delete all Che related config maps
if err := rctx.r.client.DeleteAllOf(context.TODO(), &corev1.ConfigMap{}, client.InNamespace(rctx.namespace), cheResourcesMatchingLabelsSelector); err != nil {
if err := rctx.r.nonCachingClient.DeleteAllOf(context.TODO(), &corev1.ConfigMap{}, client.InNamespace(rctx.namespace), cheResourcesMatchingLabelsSelector); err != nil {
return false, err
}

// Delete all Che related ingresses / routes
if rctx.isOpenShift {
err = rctx.r.client.DeleteAllOf(context.TODO(), &routev1.Route{}, client.InNamespace(rctx.namespace), cheResourcesMatchingLabelsSelector)
err = rctx.r.nonCachingClient.DeleteAllOf(context.TODO(), &routev1.Route{}, client.InNamespace(rctx.namespace), cheResourcesMatchingLabelsSelector)
} else {
err = rctx.r.client.DeleteAllOf(context.TODO(), &networking.Ingress{}, client.InNamespace(rctx.namespace), cheResourcesMatchingLabelsSelector)
err = rctx.r.nonCachingClient.DeleteAllOf(context.TODO(), &networking.Ingress{}, client.InNamespace(rctx.namespace), cheResourcesMatchingLabelsSelector)
}
if err != nil {
return false, err
}

// Delete all Che related persistent volumes
if err := rctx.r.client.DeleteAllOf(context.TODO(), &corev1.PersistentVolumeClaim{}, client.InNamespace(rctx.namespace), cheResourcesMatchingLabelsSelector); err != nil {
if err := rctx.r.nonCachingClient.DeleteAllOf(context.TODO(), &corev1.PersistentVolumeClaim{}, client.InNamespace(rctx.namespace), cheResourcesMatchingLabelsSelector); err != nil {
return false, err
}

Expand All @@ -215,10 +215,10 @@ func deleteKeycloakPod(rctx *RestoreContext) (bool, error) {
}
keycloakPodNsN := types.NamespacedName{Name: keycloakPodName, Namespace: rctx.namespace}
keycloakPod := &corev1.Pod{}
if err := rctx.r.client.Get(context.TODO(), keycloakPodNsN, keycloakPod); err != nil {
if err := rctx.r.nonCachingClient.Get(context.TODO(), keycloakPodNsN, keycloakPod); err != nil {
return false, err
}
if err := rctx.r.client.Delete(context.TODO(), keycloakPod); err != nil {
if err := rctx.r.nonCachingClient.Delete(context.TODO(), keycloakPod); err != nil {
return false, err
}
return true, nil
Expand Down Expand Up @@ -264,15 +264,15 @@ func restoreConfigMaps(rctx *RestoreContext, dataDir string) (bool, error) {

configMap.ObjectMeta.Namespace = rctx.namespace

if err := rctx.r.client.Create(context.TODO(), configMap); err != nil {
if err := rctx.r.nonCachingClient.Create(context.TODO(), configMap); err != nil {
if !errors.IsAlreadyExists(err) {
return false, err
}

if err := rctx.r.client.Delete(context.TODO(), configMap); err != nil {
if err := rctx.r.nonCachingClient.Delete(context.TODO(), configMap); err != nil {
return false, err
}
if err := rctx.r.client.Create(context.TODO(), configMap); err != nil {
if err := rctx.r.nonCachingClient.Create(context.TODO(), configMap); err != nil {
return false, err
}
}
Expand Down Expand Up @@ -306,15 +306,15 @@ func restoreSecrets(rctx *RestoreContext, dataDir string) (bool, error) {
secret.ObjectMeta.Namespace = rctx.namespace
secret.ObjectMeta.OwnerReferences = nil

if err := rctx.r.client.Create(context.TODO(), secret); err != nil {
if err := rctx.r.nonCachingClient.Create(context.TODO(), secret); err != nil {
if !errors.IsAlreadyExists(err) {
return false, err
}

if err := rctx.r.client.Delete(context.TODO(), secret); err != nil {
if err := rctx.r.nonCachingClient.Delete(context.TODO(), secret); err != nil {
return false, err
}
if err := rctx.r.client.Create(context.TODO(), secret); err != nil {
if err := rctx.r.nonCachingClient.Create(context.TODO(), secret); err != nil {
return false, err
}
}
Expand All @@ -329,7 +329,7 @@ func restoreCheCR(rctx *RestoreContext, dataDir string) (bool, error) {
return done, err
}

if err := rctx.r.client.Create(context.TODO(), cheCR); err != nil {
if err := rctx.r.cachingClient.Create(context.TODO(), cheCR); err != nil {
if errors.IsAlreadyExists(err) {
// We should take into account that every step can be executed several times due to async behavior.
// 1. We ensured that CheCluster is removed before restoring.
Expand Down Expand Up @@ -499,7 +499,7 @@ func getDatabaseOwner(rctx *RestoreContext, dbName string) (string, error) {
Name: rctx.cheCR.Spec.Database.ChePostgresSecret,
Namespace: rctx.namespace,
}
if err := rctx.r.client.Get(context.TODO(), chePostgresCredentialSecretNsN, secret); err != nil {
if err := rctx.r.nonCachingClient.Get(context.TODO(), chePostgresCredentialSecretNsN, secret); err != nil {
return "", err
}
return string(secret.Data["user"]), nil
Expand Down Expand Up @@ -533,7 +533,7 @@ func getPatchDatabaseScript(rctx *RestoreContext, dbName string, dataDir string)
newNamespace := rctx.namespace
appsDomain := ""
if rctx.isOpenShift {
appsDomain, err = util.GetRouterCanonicalHostname(rctx.r.client, rctx.namespace)
appsDomain, err = util.GetRouterCanonicalHostname(rctx.r.nonCachingClient, rctx.namespace)
if err != nil {
return "", err
}
Expand Down
Loading