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

Plugin flattening #240

Merged
merged 16 commits into from
Feb 3, 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
5 changes: 5 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ endif
mv config/base/config.properties.bak config/base/config.properties
mv config/base/manager_image_patch.yaml.bak config/base/manager_image_patch.yaml

### install_plugin_templates: Deploy sample plugin templates to namespace devworkspace-plugins:
install_plugin_templates: _print_vars
Copy link
Member

Choose a reason for hiding this comment

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

I'm OK with it as temporary solution until Theia did not provide DWT for its plugins. But generally I'm not sure we should have a dedicated namespace for devworkspace-plugins, and to avoid security issues we should disable cross-namespace DTW usage at this point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, this is a development feature, since we need something to replace plugin registry until something else is implemented. this rule deploys sample plugins that are compatible with devfile samples.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As for cross-namespace dwt use, I'm not sure I see the security risk necessarily. We end up running into an issue of cleanup if we're spreading dwts across the cluster, for statically created resources as we currently use.

$(K8S_CLI) create namespace devworkspace-plugins || true
$(K8S_CLI) apply -f samples/plugins -n devworkspace-plugins
amisevsk marked this conversation as resolved.
Show resolved Hide resolved

### restart: Restart devworkspace-controller deployment
restart:
$(K8S_CLI) rollout restart -n $(NAMESPACE) deployment/devworkspace-controller-manager
Expand Down
1 change: 1 addition & 0 deletions config/components/rbac/edit-workspaces-cluster-role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ rules:
- workspace.devfile.io
resources:
- devworkspaces
- devworkspacetemplates
verbs:
- create
- delete
Expand Down
1 change: 1 addition & 0 deletions config/components/rbac/view-workspaces-cluster-role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ rules:
- workspace.devfile.io
resources:
- devworkspaces
- devworkspacetemplates
verbs:
- get
- list
Expand Down
72 changes: 49 additions & 23 deletions controllers/workspace/devworkspace_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"strings"
"time"

"github.com/devfile/devworkspace-operator/pkg/library/flatten"

containerlib "github.com/devfile/devworkspace-operator/pkg/library/container"
shimlib "github.com/devfile/devworkspace-operator/pkg/library/shim"
storagelib "github.com/devfile/devworkspace-operator/pkg/library/storage"
Expand Down Expand Up @@ -123,7 +125,7 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct
// Handle stopped workspaces
if !workspace.Spec.Started {
timing.ClearAnnotations(workspace)
r.syncTimingToCluster(ctx, workspace, reqLogger)
r.syncTimingToCluster(ctx, workspace, map[string]string{}, reqLogger)
return r.stopWorkspace(workspace, reqLogger)
}

Expand All @@ -139,27 +141,28 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct
Conditions: map[devworkspace.WorkspaceConditionType]string{},
Phase: devworkspace.WorkspaceStatusStarting,
}
timing.SetTime(workspace, timing.WorkspaceStarted)
clusterWorkspace := workspace.DeepCopy()
timingInfo := map[string]string{}
timing.SetTime(timingInfo, timing.WorkspaceStarted)
defer func() (reconcile.Result, error) {
r.syncTimingToCluster(ctx, workspace, reqLogger)
return r.updateWorkspaceStatus(workspace, reqLogger, &reconcileStatus, reconcileResult, err)
r.syncTimingToCluster(ctx, clusterWorkspace, timingInfo, reqLogger)
return r.updateWorkspaceStatus(clusterWorkspace, reqLogger, &reconcileStatus, reconcileResult, err)
}()

msg, err := r.validateCreatorTimestamp(workspace)
msg, err := r.validateCreatorTimestamp(clusterWorkspace)
if err != nil {
reconcileStatus.Phase = devworkspace.WorkspaceStatusFailed
reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = msg
return reconcile.Result{}, err
}

_, ok := workspace.Annotations[config.WorkspaceStopReasonAnnotation]
if ok {
delete(workspace.Annotations, config.WorkspaceStopReasonAnnotation)
err = r.Update(context.TODO(), workspace)
if _, ok := clusterWorkspace.Annotations[config.WorkspaceStopReasonAnnotation]; ok {
delete(clusterWorkspace.Annotations, config.WorkspaceStopReasonAnnotation)
err = r.Update(context.TODO(), clusterWorkspace)
return reconcile.Result{Requeue: true}, err
}

restrictedAccess := workspace.Annotations[config.WorkspaceRestrictedAccessAnnotation]
restrictedAccess := clusterWorkspace.Annotations[config.WorkspaceRestrictedAccessAnnotation]
if restrictedAccess == "true" && config.ControllerCfg.GetWebhooksEnabled() != "true" {
reqLogger.Info("Workspace is configured to have restricted access but webhooks are not enabled.")
reconcileStatus.Phase = devworkspace.WorkspaceStatusFailed
Expand All @@ -170,10 +173,24 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct
return reconcile.Result{}, nil
}

timing.SetTime(workspace, timing.ComponentsCreated)
// TODO#185 : Move away from using devfile 1.0 constructs; only work on flattened devfiles until
// TODO#185 : plugins is figured out.
timing.SetTime(timingInfo, timing.ComponentsCreated)
// TODO#185 : Temporarily do devfile flattening in main reconcile loop; this should be moved to a subcontroller.
// TODO#185 : Implement defaulting container component for Web Terminals for compatibility
flattenHelpers := flatten.ResolverTools{
InstanceNamespace: workspace.Namespace,
Context: ctx,
K8sClient: r.Client,
}
flattenedWorkspace, err := flatten.ResolveDevWorkspace(workspace.Spec.Template, flattenHelpers)
if err != nil {
reqLogger.Info("DevWorkspace start failed")
reconcileStatus.Phase = devworkspace.WorkspaceStatusFailed
// TODO: Handle error more elegantly
reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = fmt.Sprintf("Error processing devfile: %s", err)
return reconcile.Result{}, nil
}
workspace.Spec.Template = *flattenedWorkspace

devfilePodAdditions, err := containerlib.GetKubeContainersFromDevfile(workspace.Spec.Template)
if err != nil {
reqLogger.Info("DevWorkspace start failed")
Expand All @@ -197,7 +214,7 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct
return reconcile.Result{}, nil
}
reconcileStatus.Conditions[devworkspace.WorkspaceReady] = ""
timing.SetTime(workspace, timing.ComponentsReady)
timing.SetTime(timingInfo, timing.ComponentsReady)

// Only add che rest apis if Theia editor is present in the devfile
if restapis.IsCheRestApisRequired(workspace.Spec.Template.Components) {
Expand Down Expand Up @@ -225,7 +242,7 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct
}

// Step two: Create routing, and wait for routing to be ready
timing.SetTime(workspace, timing.RoutingCreated)
timing.SetTime(timingInfo, timing.RoutingCreated)
routingStatus := provision.SyncRoutingToCluster(workspace, componentDescriptions, clusterAPI)
if !routingStatus.Continue {
if routingStatus.FailStartup {
Expand All @@ -239,9 +256,9 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct
return reconcile.Result{Requeue: routingStatus.Requeue}, routingStatus.Err
}
reconcileStatus.Conditions[devworkspace.WorkspaceRoutingReady] = ""
timing.SetTime(workspace, timing.RoutingReady)
timing.SetTime(timingInfo, timing.RoutingReady)

statusOk, err := syncWorkspaceIdeURL(workspace, routingStatus.ExposedEndpoints, clusterAPI)
statusOk, err := syncWorkspaceIdeURL(clusterWorkspace, routingStatus.ExposedEndpoints, clusterAPI)
if err != nil {
return reconcile.Result{}, err
}
Expand Down Expand Up @@ -285,7 +302,7 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct
reconcileStatus.Conditions[devworkspace.WorkspaceServiceAccountReady] = ""

// Step six: Create deployment and wait for it to be ready
timing.SetTime(workspace, timing.DeploymentCreated)
timing.SetTime(timingInfo, timing.DeploymentCreated)
deploymentStatus := provision.SyncDeploymentToCluster(workspace, podAdditions, serviceAcctName, clusterAPI)
if !deploymentStatus.Continue {
if deploymentStatus.FailStartup {
Expand All @@ -298,17 +315,17 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct
return reconcile.Result{Requeue: deploymentStatus.Requeue}, deploymentStatus.Err
}
reconcileStatus.Conditions[devworkspace.WorkspaceReady] = ""
timing.SetTime(workspace, timing.DeploymentReady)
timing.SetTime(timingInfo, timing.DeploymentReady)

serverReady, err := checkServerStatus(workspace)
serverReady, err := checkServerStatus(clusterWorkspace)
if err != nil {
return reconcile.Result{}, err
}
if !serverReady {
return reconcile.Result{RequeueAfter: 1 * time.Second}, nil
}
timing.SetTime(workspace, timing.WorkspaceReady)
timing.SummarizeStartup(workspace)
timing.SetTime(timingInfo, timing.WorkspaceReady)
timing.SummarizeStartup(clusterWorkspace)
reconcileStatus.Phase = devworkspace.WorkspaceStatusRunning
return reconcile.Result{}, nil
}
Expand Down Expand Up @@ -351,8 +368,13 @@ func (r *DevWorkspaceReconciler) stopWorkspace(workspace *devworkspace.DevWorksp
}

func (r *DevWorkspaceReconciler) syncTimingToCluster(
ctx context.Context, workspace *devworkspace.DevWorkspace, reqLogger logr.Logger) {
ctx context.Context, workspace *devworkspace.DevWorkspace, timingInfo map[string]string, reqLogger logr.Logger) {
if timing.IsEnabled() {
for timingEvent, timestamp := range timingInfo {
if _, set := workspace.Annotations[timingEvent]; !set {
workspace.Annotations[timingEvent] = timestamp
}
}
if err := r.Update(ctx, workspace); err != nil {
if k8sErrors.IsConflict(err) {
reqLogger.Info("Got conflict when trying to apply timing annotations to workspace")
Expand All @@ -375,6 +397,10 @@ func (r *DevWorkspaceReconciler) SetupWithManager(mgr ctrl.Manager) error {
// TODO: Set up indexing https://book.kubebuilder.io/cronjob-tutorial/controller-implementation.html#setup
return ctrl.NewControllerManagedBy(mgr).
For(&devworkspace.DevWorkspace{}).
// List DevWorkspaceTemplates as owned to enable updating workspaces when templates
// are changed; this should be moved to whichever controller is responsible for flattening
// DevWorkspaces
Owns(&devworkspace.DevWorkspaceTemplate{}).
Owns(&appsv1.Deployment{}).
Owns(&batchv1.Job{}).
Owns(&controllerv1alpha1.Component{}).
Expand Down
7 changes: 6 additions & 1 deletion controllers/workspace/provision/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,12 @@ func generateRBAC(namespace string) []runtime.Object {
{
Resources: []string{"devworkspaces"},
APIGroups: []string{"workspace.devfile.io"},
Verbs: []string{"patch", "get"},
Verbs: []string{"patch", "get", "update"},
},
{
Resources: []string{"devworkspacetemplates"},
APIGroups: []string{"workspace.devfile.io"},
Verbs: []string{"get", "create", "patch", "update", "delete", "list", "watch"},
},
},
},
Expand Down
3 changes: 3 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,9 @@ github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7/go.mod h1:Fecb
github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de4/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs=
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk=
github.com/grpc-ecosystem/grpc-gateway v1.9.5/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY=
github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/go-multierror v1.1.0 h1:B9UzwGQJehnUY1yNrnwREHc3fGbC2xefo8g4TbElacI=
github.com/hashicorp/go-multierror v1.1.0/go.mod h1:spPvp8C1qA32ftKqdAHm4hHTbPw+vmowP0z+KUhOZdA=
github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
Expand Down Expand Up @@ -243,6 +245,7 @@ github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0j
github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/mitchellh/reflectwalk v1.0.1 h1:FVzMWA5RllMAKIdUSC8mdWo3XtwoecrH79BY70sEEpE=
github.com/mitchellh/reflectwalk v1.0.1/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg=
Expand Down
4 changes: 2 additions & 2 deletions pkg/library/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (

devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
"github.com/devfile/devworkspace-operator/pkg/library"
"github.com/devfile/devworkspace-operator/pkg/library/flatten"
"github.com/devfile/devworkspace-operator/pkg/library/lifecycle"
)

Expand All @@ -41,7 +41,7 @@ import (
//
// Note: Requires DevWorkspace to be flattened (i.e. the DevWorkspace contains no Parent or Components of type Plugin)
func GetKubeContainersFromDevfile(workspace devworkspace.DevWorkspaceTemplateSpec) (*v1alpha1.PodAdditions, error) {
if !library.DevWorkspaceIsFlattened(workspace) {
if !flatten.DevWorkspaceIsFlattened(workspace) {
return nil, fmt.Errorf("devfile is not flattened")
}
podAdditions := &v1alpha1.PodAdditions{}
Expand Down
2 changes: 1 addition & 1 deletion pkg/library/flatten.go → pkg/library/flatten/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
// Red Hat, Inc. - initial API and implementation
//

package library
package flatten

import devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"

Expand Down
Loading