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

feat: add WorkspaceStopped method to RoutingSolver #839

Closed
wants to merge 1 commit into from
Closed

feat: add WorkspaceStopped method to RoutingSolver #839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions apis/controller/v1alpha1/devworkspacerouting_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package v1alpha1

import (
"github.com/devfile/devworkspace-operator/pkg/constants"
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand Down Expand Up @@ -205,3 +206,7 @@ type DevWorkspaceRoutingList struct {
func init() {
SchemeBuilder.Register(&DevWorkspaceRouting{}, &DevWorkspaceRoutingList{})
}

func (d *DevWorkspaceRouting) IsWorkspaceStopped() bool {
return d.Annotations != nil && d.Annotations[constants.DevWorkspaceStartedStatusAnnotation] == "false"
}
Comment on lines +210 to +212
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not 100% on whether or not the type definition files should include struct methods like this, but this one is probably fine.

17 changes: 9 additions & 8 deletions controllers/controller/devworkspacerouting/devworkspacerouting_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,15 @@ func (r *DevWorkspaceRoutingReconciler) Reconcile(ctx context.Context, req ctrl.
return reconcile.Result{}, r.finalize(solver, instance)
}

if instance.Annotations != nil && instance.Annotations[constants.DevWorkspaceStartedStatusAnnotation] == "false" {
return reconcile.Result{}, nil
workspaceMeta := solvers.DevWorkspaceMetadata{
DevWorkspaceId: instance.Spec.DevWorkspaceId,
Namespace: instance.Namespace,
PodSelector: instance.Spec.PodSelector,
}

if instance.IsWorkspaceStopped() {
err := solver.WorkspaceStopped(instance, workspaceMeta)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a case where stopping a workspace may require a second reconcile? I'm wonder if we need an error type (or to just reuse RoutingNotReady) to signify that a requeue is needed but no actual error occurred.

return reconcile.Result{}, err
}

if instance.Status.Phase == controllerv1alpha1.RoutingFailed {
Expand All @@ -120,12 +127,6 @@ func (r *DevWorkspaceRoutingReconciler) Reconcile(ctx context.Context, req ctrl.
return reconcile.Result{}, err
}

workspaceMeta := solvers.DevWorkspaceMetadata{
DevWorkspaceId: instance.Spec.DevWorkspaceId,
Namespace: instance.Namespace,
PodSelector: instance.Spec.PodSelector,
}

restrictedAccess, setRestrictedAccess := instance.Annotations[constants.DevWorkspaceRestrictedAccessAnnotation]
routingObjects, err := solver.GetSpecObjects(instance, workspaceMeta)
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions controllers/controller/devworkspacerouting/solvers/basic_solver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,7 @@ func (s *BasicSolver) GetExposedEndpoints(
routingObj RoutingObjects) (exposedEndpoints map[string]controllerv1alpha1.ExposedEndpointList, ready bool, err error) {
return getExposedEndpoints(endpoints, routingObj)
}

func (s *BasicSolver) WorkspaceStopped(routing *controllerv1alpha1.DevWorkspaceRouting, workspaceMeta DevWorkspaceMetadata) error {
return nil
}
4 changes: 4 additions & 0 deletions controllers/controller/devworkspacerouting/solvers/cluster_solver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,7 @@ func getHostnameFromService(service corev1.Service, port int32) string {
}
return fmt.Sprintf("%s://%s.%s.svc:%d", scheme, service.Name, service.Namespace, port)
}

func (s *ClusterSolver) WorkspaceStopped(routing *controllerv1alpha1.DevWorkspaceRouting, workspaceMeta DevWorkspaceMetadata) error {
return nil
}
3 changes: 3 additions & 0 deletions controllers/controller/devworkspacerouting/solvers/solver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ type RoutingSolver interface {
// Return value "ready" specifies if all endpoints are resolved on the cluster; if false it is necessary to retry, as
// URLs will be undefined.
GetExposedEndpoints(endpoints map[string]controllerv1alpha1.EndpointList, routingObj RoutingObjects) (exposedEndpoints map[string]controllerv1alpha1.ExposedEndpointList, ready bool, err error)

// WorkspaceStopped is called when the DevWorkspace for the current routing has .spec.started set to false
WorkspaceStopped(routing *controllerv1alpha1.DevWorkspaceRouting, workspaceMeta DevWorkspaceMetadata) error
}

type RoutingSolverGetter interface {
Expand Down