Skip to content
This repository has been archived by the owner on Jan 20, 2023. It is now read-only.

[WIP] Use localhost in plugin endpoints by default #56

Closed
wants to merge 2 commits into from

Conversation

davidfestal
Copy link
Contributor

What does this PR do?

Since, in the current Che implementation, all plugin containers are run on the same POD as the Theia IDE container, let's change the default behavior of the plugin brokers to use localhost as the IP of side-car plugin endpoints, instead of using the short service name, which, in some cases, cannot be reached.

What issues does this PR fix or reference?

eclipse-che/che#11018

Has this PR been tested ?

Yes, on Minishift Che Addon with the v2 prod-preview plugin registry, the nightly che-server docker image, and the modified unified plugin broker.

Signed-off-by: David Festal <dfestal@redhat.com>
Copy link

@garagatyi garagatyi left a comment

Choose a reason for hiding this comment

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

Tests are not passing. I also added an inlined comment

cfg/cfg.go Show resolved Hide resolved
Signed-off-by: David Festal <dfestal@redhat.com>
@davidfestal davidfestal changed the title Use localhost in plugin endpoints by default [WIP] Use localhost in plugin endpoints by default May 3, 2019
@davidfestal
Copy link
Contributor Author

Tests are not passing

@garagatyi I'll fix tests as soon as your last PRs are merged, since I expect a number of conflicts to occur.

@@ -31,7 +31,9 @@ func GenerateSidecarTooling(image string, pj model.PackageJSON, rand common.Rand
endpoint := generateTheiaSidecarEndpoint(rand)
setEndpoint(tooling, endpoint)
AddExtension(tooling, pj)

if cfg.UseLocalhostInPluginUrls {
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation issue here -- spaces instead of tabs

@davidfestal
Copy link
Contributor Author

A new PR will be opened from another branch after rebasing on the last master.

metlos pushed a commit to metlos/che-plugin-broker that referenced this pull request Jun 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants