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

Enable referencing plugin components by ID (internal-registry only) #263

Merged
merged 9 commits into from
Feb 9, 2021

Conversation

amisevsk
Copy link
Collaborator

@amisevsk amisevsk commented Feb 4, 2021

What does this PR do?

Implements the internal registry for components that specify a plugin by ID. This re-enables the normal web terminal flow (including defaulting the tooling container component)

What issues does this PR fix or reference?

Part of #249

Is it tested? How?

make docker install
oc apply -f samples/web-terminal.yaml

Note: opening the web terminal from the openshift UI will not work, due to #258

@amisevsk amisevsk requested a review from sleshchenko as a code owner February 4, 2021 22:17
@amisevsk amisevsk changed the title Plugin ids Enable referencing plugin components by ID (internal-registry only) Feb 4, 2021
Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

I haven't tested it but the code LGTM.

command: ["/go/bin/che-machine-exec",
"--authenticated-user-id", "$(DEVWORKSPACE_CREATOR)",
"--idle-timeout", "$(DEVWORKSPACE_IDLE_TIMEOUT)",
"--pod-selector", "controller.devfile.io/workspace_id=$(CHE_WORKSPACE_ID)",
Copy link
Member

Choose a reason for hiding this comment

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

let's start using DEVWORKSPACE_ID here and in other place instead of Che related

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍

protocol: http
attributes:
type: ide
cookiesAuthEnabled: "true"
Copy link
Member

Choose a reason for hiding this comment

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

Is it still relevant? and after we introduce POST method there, it might be not safe.
So, I just propose to remove unused stuff for time being.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, JPinkney, sleshchenko

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [JPinkney,amisevsk,sleshchenko]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot
Copy link
Collaborator

New changes are detected. LGTM label has been removed.

@amisevsk
Copy link
Collaborator Author

amisevsk commented Feb 5, 2021

/test v5-devworkspaces-operator-e2e

@amisevsk
Copy link
Collaborator Author

amisevsk commented Feb 5, 2021

/retest

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Re-implement defaulting functionality for web terminal devworkspaces;
applies a default container component to the DevWorkspace if
a) it imports the web-terminal plugin, and
b) the DevWorkspace does not contain any container components

The default container can be configured via the property

    devworkspace.default_dockerimage.redhat-developer.web-terminal

if left empty, a default container is provisioned.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
@amisevsk
Copy link
Collaborator Author

amisevsk commented Feb 8, 2021

Forgot to push my locally-rebased branch on Friday :D

@amisevsk
Copy link
Collaborator Author

amisevsk commented Feb 8, 2021

/test v5-devworkspaces-operator-e2e

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
@amisevsk
Copy link
Collaborator Author

amisevsk commented Feb 9, 2021

/retest

@amisevsk
Copy link
Collaborator Author

amisevsk commented Feb 9, 2021

/test v5-devworkspaces-operator-e2e

@amisevsk
Copy link
Collaborator Author

amisevsk commented Feb 9, 2021

The full WTO flow works with changes from eclipse-che/che-machine-exec#129 applied \o/

@amisevsk amisevsk merged commit ea42c2c into devfile:master Feb 9, 2021
@amisevsk amisevsk deleted the plugin-ids branch February 9, 2021 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants