issue #53 - adding config-map support - adding secret support - using…

[bfitzpat]

… k8s api

Signed-off-by: Brian Fitzpatrick [email protected]

[apupier]

compilation error reported:

src/ConfigMapUtils.ts:75:5 - error TS2322: Type '{}' is not assignable to type 'boolean'.
 75     return new Promise( async (resolve, reject) => {
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 76         const kubectl = await k8s.extension.kubectl.v1;
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
... 
 85         }
    ~~~~~~~~~
 86     });
    ~~~~~~~

[bfitzpat]

compilation error reported:

Yes, I am aware. That's why this is "work in progress"

[apupier]

mentioning just tab/space issue

[bfitzpat]

New menus added for creating config-maps and secrets in kubernetes using the kubectl api.

New drop-down lists added when starting a new integration. Two happen one after the other. First, the user is presented with a list of published config-maps available in the system.

If they want to use one, they select it with the mouse or keyboard and hit enter. If not, they leave the field blank.

Next up they have a drop-down list of published secrets. Again, if they want to use one they select it and move on. If not, they leave it blank.

[bfitzpat]

My concern is that to publish a simple integration without needing a config-map or secret now requires hitting additional keys or making additional selections. The "flow" is a bit erratic to be reminded of config-maps and secrets each time you start a new integration.

But if we add more menus, that becomes confusing as well. Do we then have three menus -- Start Integration, Start Integration with Config Map, Start Integration with Secret?

What happens when we then add additional resources and individual properties? That becomes two more menus...

We could possibly go with Start Integration and provide a list of options -- Simple, Properties, Resources, Config Map, and Secret -- they choose one and we go down the rabbit hole with their selection. That reduces it to a single click for a simple integration.

[bfitzpat]

Changing the flow slightly. Now we front the flow by asking the user what they are trying to accomplish (simple integration, integration with config-map, or integration with secret). Based on their selection, we route them accordingly.

A simple integration does not require any further input from the user, so it just does the deed.

Selecting Config Map moves on to enable them to select a config-map from the list of config-maps already registered in the Kubernetes system.

Same thing for Secrets.

I think this gives us the room we need to move forward with adding Property- and Resource- style deployments in additional PRs.

[apupier]

New menus added for creating config-maps and secrets in kubernetes using the kubectl api.

I think we should not put the Kubernetes action at the top of menu. It won't be the most common actions compared to classic "open". But not sure where is the best place.

Another thing which seems more important is to have a different category to not mix up with "Open/reveal" actions. But maybe best to handle it in another PR as it applies also for other Camel-K contextual menu command.

[apupier]

if not too much work, it would be nice to split the PR in 2:

• the part to create configmap and secrets
• the part improving the Camel-K integration commands using configmap and secret.

advantages:

• smaller PR to review
• the part to create configmap and secrets will be nice to contribute upstream, so picking a single PR could ease the process.

The ConfigMap file is dealing with Config AND secret, seems better to rename it.

sharing already this feedback. My minikube is corrupt and not sure when i will be able to have it get back to test more

[apupier]

why there is a cap for the H in placeHolder? I thought it would be simply placeholder as it is a "common" word.

[bfitzpat]

'placeHolder' is part of the vscode API - https://code.visualstudio.com/api/references/vscode-api#QuickPickOptions

[apupier]

not sure this comment useful.
renaming the class and the method should be enough to make it clear.

[bfitzpat]

Leaving the comment. I think it's provides additional context without a lot of overhead (a single comment line).

[apupier]

I manage to install Kubernetes locally.
The Kubernetes extension can display the perspective. But when i click on "Create Kubernetes Config Map from File", I keep seeing the message: Kubernetes extensions still coming up, please wait a moment and try again.

I don't have access to any other errors. By putting breakpoints, I noticed that it is because it doesn't find the "camel.apache.org/v1alpha1" when calling 'api-versions'. I think the error should be clearer for a normal user. (it is normal to not find it as I didn't install it)

[apupier]

there is an error when trying to create a config map with space:

Problem encountered creating new config-map named "myConfigMap with space": Error: error: exactly one NAME is required, got 4 See 'kubectl create configmap -h' for help and examples

• if not supported by kubectl, needs validation in UI
• if possible, needs to escape/enclosed value i guess

EDIT: seems not possible, the error reported is:
The ConfigMap "new config map with space" is invalid: metadata.name: Invalid value: "new config map with space": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
so UI validation is needed

[apupier]

well, having errors even with a simple name test

Problem encountered creating new config-map named "test": Error: error: exactly one NAME is required, got 2 See 'kubectl create configmap -h' for help and examples

In fact it is because the path to the file contains a space

create configmap new-config-map --from-file=c:\Users\Aurelien Pupier\Desktop\camelfiles\application.properties

using enclosed path is working

kubectl create configmap new-config-map --from-file="c:\Users\Aurelien Pupier\Desktop\camelfiles\application.properties"
configmap/new-config-map created

[bfitzpat]

I think this is a little cleaner as far as the drop-down that appears after you select "Start Integration"

[lhein]

New menus added for creating config-maps and secrets in kubernetes using the kubectl api.

I think we should not put the Kubernetes action at the top of menu. It won't be the most common actions compared to classic "open". But not sure where is the best place.

Another thing which seems more important is to have a different category to not mix up with "Open/reveal" actions. But maybe best to handle it in another PR as it applies also for other Camel-K contextual menu command.

see microsoft/vscode#9827
That feature request is open since 2016. No Sub Menus possible. Wondering if its possible to move those commands down the list a bit maybe in an own section bordered by ------------- up and down.

[lhein]

you are just checking for any content on stdout. Is that really a sufficient test to determine if the integration has been created successfully?

[bfitzpat]

Yes, because deploying a new integration takes a long time. This is why we now have decorators and a status tooltip for each "deployed" integration that shows up in the system. The state changes over time through a few states and it's better to get it out there and wait for the result.

[bfitzpat]

see microsoft/vscode#9827
That feature request is open since 2016. No Sub Menus possible. Wondering if its possible to move those commands down the list a bit maybe in an own section bordered by ------------- up and down.

I have moved them down like I did the Start Apache Camel-K Integration menu:

[bfitzpat]

@lhein thanks to your help with cleaning up the tests and promise issues, we're down to just a couple of things that ONLY crop up on Travis:

  ensure that the upstream kubernetes.go sanitize in camel-k have not changed since we checked last
rejected promise not handled within 1 second: undefined
/tmp/sanitize.go
curl -o sanitize.go -L https://raw.githubusercontent.com/apache/camel-k/master/pkg/util/kubernetes/sanitize.go
      ✓ test to see if the sanitize.go file has changed since we stashed it: 429ms
  Suite duration: 1.061 s, Tests: 1

and then at the end...

rejected promise not handled within 1 second: TypeError: Cannot read property 'stack' of undefined
stack trace: TypeError: Cannot read property 'stack' of undefined
    at t.onMessage.process.on.t.catch.t (/home/travis/build/camel-tooling/vscode-camelk/.vscode-test/vscode-1.37.0/VSCode-linux-x64/resources/app/out/vs/workbench/services/extensions/node/extensionHostProcess.js:775:578)

I'm not seeing anything obvious on the sanitize.go test, but would be happy with the improved stability we have now and could spin that off into #69

[bfitzpat]

Took care of one more promise rejected message that was showing up at runtime in the console. Woot! Still have two left in the tests, but we're getting there @lhein

[lhein]

Reviewed as good as possible. Due to not having a working camelk install locally I wasn't able to test run it but Brian provided a video showing functionality which looked good. Remaining rejected promises can be handled in #69.