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

feat(core): Add VS Camel K extension eclipse/che#14806 #247

Merged
merged 1 commit into from
Oct 11, 2019

Conversation

apupier
Copy link
Contributor

@apupier apupier commented Oct 9, 2019

  • requires a specific dockerfile with Kubectl and Kamel on cli.
  • Add Kubernetes extension which is a required dependency of VS Code
    Camel K tooling

Signed-off-by: Aurélien Pupier apupier@redhat.com

What does this PR do?

Add VS Code Tooling for Apache Camel in Che Plugin registry

/!\ it requires this other PR to be merged with Dockerfile published: eclipse-che/che-theia#476 Dockerfile published

- requires a specific dockerfile with Kubectl and Kamel on cli.
- Add Kubernetes extension which is a required dependency of VS Code
Camel K tooling

Signed-off-by: Aurélien Pupier <apupier@redhat.com>
Copy link
Contributor

@nickboldt nickboldt left a comment

Choose a reason for hiding this comment

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

LGTM, but maybe another person should review too

@apupier
Copy link
Contributor Author

apupier commented Oct 11, 2019

required docker image is now available https://hub.docker.com/r/eclipse/che-remote-plugin-camelk-0.0.9

@nickboldt nickboldt self-requested a review October 11, 2019 12:15
Copy link
Contributor

@nickboldt nickboldt left a comment

Choose a reason for hiding this comment

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

-1, we're supposed to be migrating to Quay, not using dockerhub. Can you publish your required sidecar image there instead? and how is the image defined? Where's it's Dockerfile? Should it be in https://github.com/eclipse/che-theia/tree/master/dockerfiles with the rest of the sidecars?

firstPublicationDate: '2019-07-10'
spec:
containers:
- image: "eclipse/che-remote-plugin-camelk-0.0.9:next"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use quay.io and in general we should always include the registry here, not just the org/image:tag.

firstPublicationDate: '2019-07-10'
spec:
containers:
- image: "eclipse/che-remote-plugin-camelk-0.0.9:next"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use quay.io and in general we should always include the registry here, not just the org/image:tag.

@benoitf
Copy link
Contributor

benoitf commented Oct 11, 2019

@nickboldt che-theia builds are still publishing to docker hub (there is an other issue to move to quay.io)

@apupier
Copy link
Contributor Author

apupier commented Oct 11, 2019

-1, we're supposed to be migrating to Quay, not using dockerhub. Can you publish your required sidecar image there instead?

I prefer to stay on current system. As Florent mentioned, i think it will be handled by another specific task eclipse-che/che#14849

and how is the image defined? Where's it's Dockerfile? Should it be in https://github.com/eclipse/che-theia/tree/master/dockerfiles with the rest of the sidecars?

the Dockerfile is available herewith the rest of the sidecars
https://github.com/eclipse/che-theia/tree/master/dockerfiles/remote-plugin-camelk-0.0.9

Copy link
Contributor

@nickboldt nickboldt left a comment

Choose a reason for hiding this comment

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

OK I'll submit a second PR to fix your eclipse/ -> docker.io/eclipse/ as I see the same problem for a few other plugins. Need that prefix to make it easier to convert to airgap mode.

@nickboldt nickboldt merged commit 3c292d9 into eclipse-che:master Oct 11, 2019
@apupier apupier deleted the 14806-camelk branch October 11, 2019 13:41
@benoitf
Copy link
Contributor

benoitf commented Oct 11, 2019

@nickboldt the build.sh script should enforce that so the PR is failing if there is no organization

@nickboldt
Copy link
Contributor

Followup for approval #251

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants