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

Remove unused code for devspace endpoints #476

Merged
merged 11 commits into from
Oct 13, 2023
Merged

Remove unused code for devspace endpoints #476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2b4e06c
Remove unused code for devspace endpoints
baijum Oct 4, 2023
0b51cb7
Remove unused functions
baijum Oct 5, 2023
eb30e14
Removed che package
baijum Oct 9, 2023
7463db4
Merge remote-tracking branch 'upstream/master' into devspace-endpoint…
baijum Oct 9, 2023
6db4533
Fix lint errors
baijum Oct 9, 2023
fb7601b
Merge branch 'master' into devspace-endpoints-unused-code
MatousJobanek Oct 11, 2023
98eec49
Remove unused code based on review comment
baijum Oct 11, 2023
6dbee14
Merge branch 'master' into devspace-endpoints-unused-code
rajivnathan Oct 12, 2023
2393adf
Remove leftover function
baijum Oct 12, 2023
795fc70
Remove gock related code
baijum Oct 12, 2023
2b427c4
Merge branch 'master' into devspace-endpoints-unused-code
baijum Oct 12, 2023
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
53 changes: 0 additions & 53 deletions controllers/memberstatus/memberstatus_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
membercfg "github.com/codeready-toolchain/member-operator/controllers/memberoperatorconfig"
"github.com/codeready-toolchain/member-operator/pkg/che"
"github.com/codeready-toolchain/member-operator/version"
commonclient "github.com/codeready-toolchain/toolchain-common/pkg/client"
"github.com/codeready-toolchain/toolchain-common/pkg/cluster"
Expand Down Expand Up @@ -63,7 +62,6 @@ type Reconciler struct {
Scheme *runtime.Scheme
GetHostCluster func() (*cluster.CachedToolchainCluster, bool)
AllNamespacesClient client.Client
CheClient *che.Client
VersionCheckManager status.VersionCheckManager
}

Expand Down Expand Up @@ -125,7 +123,6 @@ func (r *Reconciler) aggregateAndUpdateStatus(reqLogger logr.Logger, memberStatu
{name: hostConnectionTag, handleStatus: r.hostConnectionHandleStatus},
{name: resourceUsageTag, handleStatus: r.loadCurrentResourceUsage},
{name: routesTag, handleStatus: r.routesHandleStatus},
{name: cheTag, handleStatus: r.cheHandleStatus},
}

// Track components that are not ready
Expand Down Expand Up @@ -306,50 +303,6 @@ func (r *Reconciler) routesHandleStatus(reqLogger logr.Logger, memberStatus *too
return nil
}

// cheHandleStatus checks all necessary aspects related integration between the member operator and Che
// Returns an error if any problems are discovered.
func (r *Reconciler) cheHandleStatus(_ logr.Logger, memberStatus *toolchainv1alpha1.MemberStatus, config membercfg.Configuration) error {
if memberStatus.Status.Che == nil {
memberStatus.Status.Che = &toolchainv1alpha1.CheStatus{}
}

// Is che user deletion enabled
if !config.Che().IsUserDeletionEnabled() {
// Che user deletion is not enabled, set condition to Ready. No further checks required
readyCondition := status.NewComponentReadyCondition(toolchainv1alpha1.ToolchainStatusMemberStatusCheUserDeletionNotEnabledReason)
memberStatus.Status.Che.Conditions = []toolchainv1alpha1.Condition{*readyCondition}
return nil
}

if !r.isCheAdminUserConfigured(config) {
err := fmt.Errorf("the Che admin user credentials are not configured but Che user deletion is enabled")
errCondition := status.NewComponentErrorCondition(toolchainv1alpha1.ToolchainStatusMemberStatusCheAdminUserNotConfiguredReason, err.Error())
memberStatus.Status.Che.Conditions = []toolchainv1alpha1.Condition{*errCondition}
return err
}

// Ensure it's possible to construct the che URL for using the Che user API
if _, err := r.cheDashboardURL(config); err != nil {
wrappedErr := errs.Wrapf(err, "Che dashboard URL unavailable but Che user deletion is enabled")
errCondition := status.NewComponentErrorCondition(toolchainv1alpha1.ToolchainStatusMemberStatusCheRouteUnavailableReason, wrappedErr.Error())
memberStatus.Status.Che.Conditions = []toolchainv1alpha1.Condition{*errCondition}
return err
}

// User API check (not applicable after migration from Che to Dev Spaces)
if !config.Che().IsDevSpacesMode() {
if err := r.CheClient.UserAPICheck(); err != nil {
errCondition := status.NewComponentErrorCondition(toolchainv1alpha1.ToolchainStatusMemberStatusCheUserAPICheckFailedReason, err.Error())
memberStatus.Status.Che.Conditions = []toolchainv1alpha1.Condition{*errCondition}
return err
}
}

readyCondition := status.NewComponentReadyCondition(toolchainv1alpha1.ToolchainStatusMemberStatusCheReadyReason)
memberStatus.Status.Che.Conditions = []toolchainv1alpha1.Condition{*readyCondition}
return nil
}

func (r *Reconciler) getAllocatableValues(reqLogger logr.Logger) (map[string]nodeInfo, error) {
nodes := &corev1.NodeList{}
err := r.Client.List(context.TODO(), nodes)
Expand Down Expand Up @@ -459,9 +412,3 @@ func sanitizeURL(url string) string {
}
return url
}

// isCheAdminUserConfigured returns true if the Che admin username and password are both set and not empty.
// Returns false otherwise.
func (r *Reconciler) isCheAdminUserConfigured(config membercfg.Configuration) bool {
return config.Che().AdminUserName() != "" && config.Che().AdminPassword() != ""
}
254 changes: 27 additions & 227 deletions controllers/memberstatus/memberstatus_controller_test.go
View file
Open in desktop

Large diffs are not rendered by default.

69 changes: 4 additions & 65 deletions controllers/useraccount/useraccount_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
membercfg "github.com/codeready-toolchain/member-operator/controllers/memberoperatorconfig"
"github.com/codeready-toolchain/member-operator/pkg/che"
commoncontroller "github.com/codeready-toolchain/toolchain-common/controllers"
"github.com/codeready-toolchain/toolchain-common/pkg/condition"
commonconfig "github.com/codeready-toolchain/toolchain-common/pkg/configuration"
Expand Down Expand Up @@ -50,9 +49,8 @@ func (r *Reconciler) SetupWithManager(mgr manager.Manager) error {

// Reconciler reconciles a UserAccount object
type Reconciler struct {
Client client.Client
Scheme *runtime.Scheme
CheClient *che.Client
Client client.Client
Scheme *runtime.Scheme
}

//+kubebuilder:rbac:groups=toolchain.dev.openshift.com,resources=useraccounts,verbs=get;list;watch;create;update;patch;delete
Expand Down Expand Up @@ -108,7 +106,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
return reconcile.Result{}, err
}
} else {
return reconcile.Result{}, r.ensureUserAccountDeletion(logger, config, userAcc)
return reconcile.Result{}, r.ensureUserAccountDeletion(logger, userAcc)
}

if !userAcc.Spec.Disabled {
Expand Down Expand Up @@ -176,19 +174,14 @@ func (r *Reconciler) ensureUserAndIdentity(logger logr.Logger, userAcc *toolchai
return false, nil
}

func (r *Reconciler) ensureUserAccountDeletion(logger logr.Logger, config membercfg.Configuration, userAcc *toolchainv1alpha1.UserAccount) error {
func (r *Reconciler) ensureUserAccountDeletion(logger logr.Logger, userAcc *toolchainv1alpha1.UserAccount) error {
if util.HasFinalizer(userAcc, toolchainv1alpha1.FinalizerName) {
logger.Info("terminating UserAccount")
// We need to be sure that the status is updated when the UserAccount is deleted.
// In this case the UserAccountStatus controller updates the MUR on the host cluster
// In turn, the MUR controller may decide to recreate the UserAccount resource on the
// member cluster.

// Clean up Che resources by deleting the Che user (required for GDPR and reactivation of users)
if err := r.lookupAndDeleteCheUser(logger, config, userAcc); err != nil {
return r.wrapErrorWithStatusUpdate(logger, userAcc, r.setStatusTerminating, err, "failed to delete Che user data")
}

deleted, err := r.deleteIdentityAndUser(logger, userAcc)
if err != nil {
return r.wrapErrorWithStatusUpdate(logger, userAcc, r.setStatusTerminating, err, "failed to delete user/identity")
Expand Down Expand Up @@ -664,60 +657,6 @@ func ToIdentityName(userID string, identityProvider string) string {
return fmt.Sprintf("%s:%s", identityProvider, userID)
}

func (r *Reconciler) lookupAndDeleteCheUser(logger logr.Logger, config membercfg.Configuration, userAcc *toolchainv1alpha1.UserAccount) error {

// If Che user deletion is not required then just return, this is a way to disable this Che user deletion logic since
// it's meant to be a temporary measure until Che is updated to handle user deletion on its own
if !config.Che().IsUserDeletionEnabled() {
logger.Info("Che user deletion is not enabled, skipping it")
return nil
}

if config.Che().IsDevSpacesMode() {
return r.deleteDevSpacesUser(logger, userAcc)
}

userExists, err := r.CheClient.UserExists(userAcc.Name)
if err != nil {
return err
}

// If the user doesn't exist then there's nothing left to do.
if !userExists {
logger.Info("Che user no longer exists")
return nil
}

// Get the Che user ID to use in the Delete API
cheUserID, err := r.CheClient.GetUserIDByUsername(userAcc.Name)
if err != nil {
return err
}

// Delete the Che user. It is common for this call to return an error multiple times before succeeding
if err := r.CheClient.DeleteUser(cheUserID); err != nil {
return errs.Wrapf(err, "this error is expected if deletion is still in progress")
}

return nil
}

func (r *Reconciler) deleteDevSpacesUser(logger logr.Logger, userAcc *toolchainv1alpha1.UserAccount) error {
logger.Info("Deleting OpenShift Dev Spaces user")

// look up user resource to get UID
userList, err := getUsersByOwnerName(r.Client, userAcc.Name)
if err != nil {
return err
}

if len(userList) == 0 {
return nil
}

return r.CheClient.DevSpacesDBCleanerDelete(string(userList[0].GetObjectMeta().GetUID()))
}

// listByOwnerLabel returns runtimeclient.ListOption that filters by label toolchain.dev.openshift.com/owner equal to the given owner name
func listByOwnerLabel(owner string) client.ListOption {
labels := map[string]string{toolchainv1alpha1.OwnerLabelKey: owner}
Expand Down
Loading
Loading