issue with camelk version check and downloading newer version

lhein commented 4 years ago

I experienced the issue that I had a kamel executable in my /usr/bin folder which was 1.0.0-M3. The vs code plugin downloads the latest camelk version and unpacks it in a vscode user folder hidden in the users home folder. When starting the plugin the check for a new version found a newer version than the one in my /usr/bin folder and downloaded it to the internal folder. On next startup of the plugin it still found the older kamel executable in /usr/bin first so it again started to download the new version. That is repeatable forever. So I guess there might be additional logic needed to prevent this to happen. We should also make sure that once you downloaded a new camelk version it is then used as the default executable for CLI commands and not the one in /usr/bin or whatever place is found first in PATH.

apupier commented 4 years ago

please raise on JIRA tracker which allows better tracking

bfitzpat commented 4 years ago

Definitely a scenario we didn't predict. Essentially the setup is this:

When the extension starts, we check to see if we need an update: https://github.com/camel-tooling/vscode-camelk/blob/9890a113e1aad3fd0646b24e934ee3134e5bf628/src/extension.ts#L348
That essentially gets the current runtime version setting from the user settings, tests to see if that version is available (i.e. we can download it). If not, we get the latest published version from https://api.github.com/repos/apache/camel-k/releases/latest (not a version tagged as preview).

We haven't installed anything yet, we're just doing some version checking.

If the version in the settings and the latest default version (set as a constant right now to 1.0.0-RC2) don't match we try to use the one in the settings. If "autoupdate" is turned off, it's user-specified. If "autoupdate" is turned on, we use the default version from the constant.
At this point, we know what version we should use, so we check to see if the kamel cli is there to return a version. This is where we run into problems in the scenario you've described.

If the nightly build is installed, it is returning 1.0.0-RC3.

And lastly, we have a test. https://github.com/camel-tooling/vscode-camelk/blob/9890a113e1aad3fd0646b24e934ee3134e5bf628/src/versionUtils.ts#L76

If the version coming from the user's workspace (in this case 1.0.0-RC2) matches the version from the kamel CLI (in this case 1.0.0-RC3), then we're golden. We don't need to try and download anything. Unfortunately, that's not the case -- so it goes ahead and downloads every time.

So it seems we need to have another check or at least put some errors or warnings in front of the user stating that the kamel CLI on the path has a different version than the one we recommend.

bfitzpat commented 4 years ago

So the question becomes... do we rely on the user's install and whatever is on the PATH to be right? If so, we default to that. If not, we download and manage the version ourselves.

apupier commented 4 years ago

do we rely on the user's install and whatever is on the PATH to be right?

this can be a preference, so we don't make a hard choice for the users. We still need to choose the default value.

bfitzpat commented 4 years ago

Regardless of whatever we put in as a preference, I think we need to be more transparent and put the version being used in the output channel for the extension, just so they KNOW when this kind of thing happens.

bfitzpat commented 4 years ago

Since we have an additional constraint when the Camel K runtime is productized and it will not be private (can only be accessed after logging in with the browser on the ocp console), we really need to take a slightly different approach and assume that any CLI we find installed before the tooling-installed one is a good one. But we should also provide some additional diagnostic to the user in the Camel K output channel to let them know that's what we're doing so we avoid the brief panic we had when Lars hit this issue initially.

So the new order of precedence for the kamel CLI will be...

if we find a version of the cli installed already, we go with it
if we don't, we download and manage it in a tooling directory

But either way, we provide some feedback to the user identifying the version of the CLI they are using in the tooling.

(We came to this approach after discussing with Nicola)

bfitzpat commented 4 years ago

Adding a check to see if the CLI is already there. In this case, we found 1.0.0-RC2 already.

"Using Camel K CLI version 1.0.0-RC2 found on the system path. If you want to use a different version, you may need to disable this one first and configure the CLI version downloaded via the extension."

bfitzpat commented 4 years ago

Melissa helped me revise it a bit... "Using Camel K CLI version 1.0.0-RC2 found on the system path. If you want to use the CLI version downloaded by the Tooling for Apache Camel K extension instead, you must disable the existing CLI. See the extension readme for more information."

We can put some details in the readme on ways to disable the existing CLI (like renaming it to something other than kamel for example). But I'm not sure we want to tell them how to disable it.

Opinions? :)

bfitzpat commented 4 years ago

We could even tell them where it is to disable it if that would help. We have https://github.com/camel-tooling/vscode-camelk/blob/9890a113e1aad3fd0646b24e934ee3134e5bf628/src/kamel.ts#L158 which can provide that information I think.

apupier commented 4 years ago

Opinions?

I think we should not ask to disable kamel on CLI to be able to use the one provided by VS Code. It is extra work for users. Depending on OS it can come from very different places. It can be hard to do. Also, it means that users won't be able to use command-lien anymore. How can they workaround a missing feature from VS Code extension? They would need to reenable the CLI. It won't be convenient at all.

I would really prefer a preference that can be turned on/off to let user choose between use the CLI or use the VS Code embedded one. It is a lot less intrusive to the Operating System of the user.

bfitzpat commented 4 years ago

I would really prefer a preference that can be turned on/off to let user choose between use the CLI or use the VS Code embedded one. It is a lot less intrusive to the Operating System of the user.

Opinion noted.

To answer a few implied questions though:

1) We are not disabling kamel on CLI to use the one provided by VS Code. Exactly the opposite. We are now defaulting to whatever kamel they have currently in the system path and only installing one in the tooling global config directory if it's not available.

2) If the CLI is installed in the tooling config directory, the user has never been able to use it at the command line. They would need to add it explicitly to their system path to pull that off.

3) If they have the CLI already working on their system, we are going to assume they know how a) it got there and b) how to update it when the need arises (if ever).

So I don't think we want to add a preference for this at all.

If kamel is on the system path when they install the Camel K extension, we will now use it more easily and NOT install anything else. If not, we will install a version in a known location and use that. I think that makes more sense in the long run.

bfitzpat commented 4 years ago

First cut of the changes are here - https://github.com/camel-tooling/vscode-camelk/pull/370

Not sure what kind of test we can provide for this however, since we aren't really testing on a system where the CLI will be pre-installed.

apupier commented 4 years ago

the sentence

If you want to use the CLI version downloaded by the Tooling for Apache Camel K extension instead, you must disable the existing CLI."

seems to not be in phase in what you are trying to achieve.

Suggesting to modify the OS configuration to use a specific version provided by the tooling seems a no go from my point of view. it is a bit like if in Eclipse if you want to use Maven 3.6.3 which is provided by Eclipse, you are asking to remove the maven that you have installed on your system.

lhein commented 4 years ago

How about a middle approach. We introduce a preference for which Kamel to use. Lets call the selectable options "Embedded" and "External".

"Embedded" : Will download (if not yet there) the Kamel like its done now already from Github and use it from there.

"External": The Default. It will use the Kamel from the PATH. No downloads will be done and the embedded Kamel will not be used.

Extra functionality: When the user tries to use kamel and "External" is selected, we check for the executable on the PATH. If found all is fine and it will be used. If its not there, then we will inform the user about the missing kamel installation and ask him if he wants to switch to the embedded version. If he selects no, the operation will be aborted. If he selects yes, the kamel is downloaded from github and then used. Maybe inform user about the download process.

Does that sound fair?

bfitzpat commented 4 years ago

This is what I wrote in the readme:

Apache Camel K CLI Installation

If you have pre-installed the Apache Camel K CLI (kamel), it is now used by the tooling.

If the CLI has not been pre-installed, the Tooling for Apache Camel K extension will install it in the global configuration area of Visual Studio Code. If the tooling manages the CLI, you have control over what version of the CLI to use and whether to auto-upgrade to the latest version in future extension updates in the settings.

If the CLI was pre-installed and you want to change versions, you will have to do that yourself or remove/rename the existing CLI so the extension knows to manage it for you.

Note: To check which version is currently installed on your system, you can use the Retrieve version for current Apache Camel K CLI command to display the version in the Apache Camel K output channel.

MelissaFlinn commented 4 years ago

Is it possible for the Tooling for Apache Camel K extension installation script to automatically check to see if there is a currently installed Apache Camel K CLI (kamel).

If there is, then prompt the user to ask if they want to use the existing one or install a new one.
If not, just install it (and also let the user know where it is now installed)

bfitzpat commented 4 years ago

@MelissaFlinn We can definitely check, but we don't really have any control over the user's system beyond the scope of their VS Code installation directories. We really don't want to make changes to their OS for them. That's part of the problem.

I kind of like @lhein 's approach of coming up with consistent terminology, but I don't know that it helps enough to clarify the situation.

Ultimately if they have the CLI installed before they install the Camel K tooling, we use it. If not, we download it for them in the realm of their VS Code install.

apupier commented 4 years ago

We really don't want to make changes to their OS for them. That's part of the problem.

I don't understand your concern on this point on Melissa (and even Lars) proposals. In none of them, it will lead to modify the Operating System.

If they have the CLI already working on their system, we are going to assume they know how a) it got there and b) how to update it when the need arises (if ever).

I'm not sure we can assume they know. I often have old cli versions of kubectl, oc, or this kind of things. And I always forget what is the version or even if I installed it already. i'm pretty sure a lot users don't remember all the CLI and in which versions that they have.

I think that Lars and Melissa proposals are very similar. It is more that Melissa is describing the easy flow for the user and Lars is starting from the technical side. But the extra functionality, is in fact the core of the flow defined by Melissa. And the flow defined by Melissa can technically be based on Lars proposal.

Lars and Melissa proposals are going a step further than what Brian is proposing where we are defaulting to command-lien without letting the choice to the user. It has the big drawback that a user cannot use a different version of kamel in VS Code extension than what is on his/her system. Which is forcing her/him to modify the Operating System configuration her/him-self. Given this drawback, I think we should go to the Lar and Melissa state directly.

MelissaFlinn commented 4 years ago

Is there a way to have both CLIs installed on a user's system (both default and embedded), and have a parameter or switch in the extension that points to the one that the extension uses?

apupier commented 4 years ago

Is there a way to have both CLIs installed on a user's system (both default and embedded), and have a parameter or switch in the extension that points to the one that the extension uses?

yes. this is the proposal of Lars

MelissaFlinn commented 4 years ago

oh - Sorry if I am misreading. I interpreted "(if not yet there)" as meaning there is no default installation. Lars's description does not seem to indicate what happens if the user selects "embedded" and the CLI is already there.

I'm wondering if the extension can go ahead and install its version of the CLI no matter what. And if there are two versions (embedded and default) then there is a switch or parameter in the extension that the user can specify at installation (and can later change).

So they are not changing an OS configuration, they are just changing a parameter value.

bfitzpat commented 4 years ago

@apupier I really don't want to have multiple copies of the CLI if we can avoid it.

I think we want to maybe take a page from the Kubernetes tooling from Microsoft here and maybe just duplicate it. https://github.com/Azure/vscode-kubernetes-tools#dependencies

The paragraph here is important:

We recommend you install these binaries on your system PATH before using the extension. If these binaries aren't on your system PATH, then some commands may not work. If the extension needs one of the core Kubernetes tools and it's missing, it will offer to install it for you.

bfitzpat commented 4 years ago

I want to voice a concern about some of the suggestions here. In my opinion, we should always adapt to the user's system wherever possible, especially in cases where runtime commands and systems need to be installed and running for us to actually do anything in the tooling itself. So going with a system path-first approach is absolutely what we should be going for here.

And I want to make it absolutely clear that that was NOT the approach we have taken so far in this extension due to a bit of shortsightedness on my part. Lars' discovered a hole in my logic. Though installing the CLIs through the tooling is expedient, we should always have been relying on the user's system configuration first.

So the change for this issue is critical as we move forward and should inform other, similar decisions down the line. I definitely want to make sure we get it right this time.

The logic that I apply in https://github.com/camel-tooling/vscode-camelk/pull/370 at extension start-up time is as follows:

Check the system path for the kamel command. If we find it, we're golden. Display that fact to the user in the Camel K output channel so they have a record of the version they are currently using.
If we don't have it, install the kamel command for them in the VS Code global configuration area so it's in a known place where we can get to it. This version is NOT added to the system path, but a savvy user can find it and add it if they so desire.
If they later install a version of kamel on the system path and the extension has handled option 2 previously, see option 1. If it's on the system path, that's the one we use over the one that we download.

If the user goes with option 1, that's the end of the discussion. But if the user goes with option 2, we have additional capabilities for managing the version used in the user settings for the extension.

(edited to get rid of an errant link created by markdown to issue #2 that shouldn't have been there)

bfitzpat commented 4 years ago

Unless we come up with good reason not to, I seriously think we should adopt the approach taken by the Kubernetes Tools folks as I detailed here - https://github.com/camel-tooling/vscode-camelk/issues/369#issuecomment-608490775

MelissaFlinn commented 4 years ago

I think that this makes sense.... Description: The Tooling for Apache Camel K extension requires kamel, the Camel K command line interface (CLI). If you already have kamel installed on your system PATH, the extension will use it. If you do not already have kamel installed on your system, the extension downloads and installs it.

Note that if you later remove the pre-installed kamel from your system PATH, the Tooling for Apache Camel K extension automatically detects that it is missing and downloads and installs its own version.

The UI would have something like:

This extension will use the Camel K CLI (kamel) installation found here: xyz
No Camel K CLI (kamel) installation detected. This extension will install kamel here: xyz

bfitzpat commented 4 years ago

I like that approach. Thank you @MelissaFlinn ! WDYT @apupier and @lhein ?

lhein commented 4 years ago

Actually I like the approach described in https://github.com/camel-tooling/vscode-camelk/issues/369#issuecomment-608490775.

We should check with Paolo / Gary if that is an option. It will clearly reduce complexity on our side and its pretty straight forward and easy to understand. Just in case we don't detect a kamel in PATH we should maybe ask the user if we should open a browser with the download page of kamel.

If thats not ok then I am fine with your latest described approach, Brian.

apupier commented 4 years ago

we are missing the case that a user wants to use a specific version of kamel in the tooling which is different from the one that is available on the command-line. It is a basic feature for most other (almost) mature project.

For instance, in Eclipse IDE, this is why there are "Installed JREs", "Maven Installations". In VS Code, "Java Execution Runtimes", "Typescript tsdk".

it also means that for Che end-users, they are blocked with a version of kamel unless they are recreating their own docker file and their plugin with their own devfile, instead of just changing a preference which will be persisted. (or that we stop embedding a kamel runtime and it is downloaded on each start of the workspace which seems a relatively big regression)

So all of you are ok to miss this feature?

bfitzpat commented 4 years ago

So all of you are ok to miss this feature?

Yes. I have been following the lead of the Kubernetes tools team from Microsoft on this. https://marketplace.visualstudio.com/items?itemName=ms-kubernetes-tools.vscode-kubernetes-tools As I pointed out earlier, the Kubernetes tools even less configuration options. You get the option of either going with what's installed on the system or they download it for you. You don't get to choose the version, it automatically grabs the most recent stable published version. So we're already going above and beyond that a bit for the version we're downloading.

I already pointed to this disclaimer in their readme:

We recommend you install these binaries on your system PATH before using the extension. If these binaries aren't on your system PATH, then some commands may not work. If the extension needs one of the core Kubernetes tools and it's missing, it will offer to install it for you.

It is a basic feature for most other (almost) mature project.

Are you saying that you don't believe that their extension is mature because it doesn't have this additional functionality? I completely disagree since it's been a 1.0.0 version since May 2019 and has nearly 500,000 installs with a five star rating in the VS Code Marketplace.

it also means that for Che end-users, they are blocked with a version of kamel...

How are we blocked for Che exactly? Every image you have had to create has included a specific version of the kamel runtime cli, hasn't it? I don't see this new approach as conflicting with that. In fact, it works WITH it. We're no longer trying to download another version on top of the one specified in your image.

So from my perspective we are not blocking anything. If they want to manage the version inside VS Code, they can do that by disabling the CLI in their system path.

// another point

I think we need to look at it from a sustainability standpoint. If the user has a version of kamel on their system path, we should use it. End of story. It's there for a reason, whether in the docker image or installed on the local system. And usually that implies that it's tied to a particular runtime -- whether it's kubernetes or openshift or something else. I think we have to assume that if it's there, the user knows it's there and that's the version they want to see used in the tooling.

As soon as we start providing two options... either the system path or the one in the vscode directory... and allow the user to choose which version they want to use, things explode. Are we then adding the tooling one to the system path? If not, the tooling version and the system path are then out of sync meaning there may be configuration clashes in the runtime when we use one version and it uses another. We really don't want to do that.

So I repeat what I said before - we really don't want to get into managing the user's system path from our extension. It's a really bad precedent to set.

// to sum up...

In my professional opinion, I think we need to go with the precedent set by the Kubernetes tools team at Microsoft. Use the one on the system path as the right one if it's there and download one to the tooling path if it's not. If they later install one on the system path, that's then the one we use over the downloaded one.

Any other scenario gets into nightmare territory.

apupier commented 4 years ago

So all of you are ok to miss this feature?

Yes. I have been following the lead of the Kubernetes tools team from Microsoft on this. https://marketplace.visualstudio.com/items?itemName=ms-kubernetes-tools.vscode-kubernetes-tools As I pointed out earlier, the Kubernetes tools even less configuration options. You get the option of either going with what's installed on the system or they download it for you. You don't get to choose the version, it automatically grabs the most recent stable published version. So we're already going above and beyond that a bit for the version we're downloading.

I think you missed some settings https://github.com/Azure/vscode-kubernetes-tools#extension-settings

vs-kubernetes.kubectl-path - File path to the kubectl binary. Note this is the binary file itself, not just the directory containing the file. On Windows, this must contain the .exe extension.

and

vs-kubernetes.kubectlVersioning - By default, the extension uses the kubectl binary you provide on the system PATH or in the vs-kubernetes.kubectl-path configuration setting. If you set this setting to infer, then for each cluster the extension will attempt to identify the cluster version and download a compatible kubectl binary. This improves compatibility if you have multiple Kubernetes versions in play, but may be slower. Note: this setting is checked only when the extension loads; if you change it, you must reload the extension.

They allow to point to a different version than what is installed on the PATH. They even allow to have automatic detection and use a different version depending on the cluster it is connecting to. Which is ultimately what would be interesting fro kamel as the kamel CLI for now requires to be in the exact same version than the cluster form most of the actions. (but that's right that we are still pre-1.0.0 so maybe it will be more stable in the future)

It is a basic feature for most other (almost) mature project.

Are you saying that you don't believe that their extension is mature because it doesn't have this additional functionality? I completely disagree since it's been a 1.0.0 version since May 2019 and has nearly 500,000 installs with a five star rating in the VS Code Marketplace.

as mentioned few lines above, Kubernetes extension have the feature.

it also means that for Che end-users, they are blocked with a version of kamel...

How are we blocked for Che exactly? Every image you have had to create has included a specific version of the kamel runtime cli, hasn't it? I don't see this new approach as conflicting with that. In fact, it works WITH it. We're no longer trying to download another version on top of the one specified in your image.

So from my perspective we are not blocking anything. If they want to manage the version inside VS Code, they can do that by disabling the CLI in their system path.

in Che, users cannot deactivate the version on the system path without rebuilding a full stack. This is a problem. On several clusters, the update of the Camel K instance is upgraded automatically, or at least outside of control of developers. it can upgrade at different pace depending on the cluster you are working on. it means that a workspace which was working one day can be broken the day after without the possibility to "fix"' the workspace without rebuilding the whole stack. Most of Che users don't know how to do that. They might even not have the right to do so.

They Must have a way to use another version than what is on the system path to not be blocked.

So I repeat what I said before - we really don't want to get into managing the user's system path from our extension. It's a really bad precedent to set.

none of the proposed solution mentioned managing the user system path.

n my professional opinion, I think we need to go with the precedent set by the Kubernetes tools team at Microsoft. Use the one on the system path as the right one if it's there and download one to the tooling path if it's not. If they later install one on the system path, that's then the one we use over the downloaded one.

As explained, this is not what they are doing. I just tried with the following scenario:

have kubectl on path
set vs-kubernetes.kubectl-path to a dummy value such as /awrongpath/kubectl
there is an erro message /bin/sh: /awrongpath/kubectl: No such file or directory --> it means that when a setting ios specific to use another version, this is the one that is used and not the on from the system path.

bfitzpat commented 4 years ago

User cases discussed in the meeting:

User settings-specified path to binary. (We don't have this currently configurable.)
Binary on system path.
Download a version based on the version specified in the user settings.

make sure we revisit this when we have a productized version

bfitzpat commented 4 years ago

Working on a settings drop-down to provide a bit better focus on the method to use to find the Apache Camel K binary...

Three options each have their own description that shows up at the bottom when selected:

system-path : "System path includes the version of the Apache Camel K binary to use"
user-specified-path" : "Path to Apache Camel K binary is user-specified in the to-be-named setting"
tooling-managed : "Tooling downloads and manages Apache Camel K binary based on user-specified version in the Default Apache Camel K Runtime Version setting"

bfitzpat commented 4 years ago

Stashing this here so I can move to a different issue:

    "camelk.binary" : {
        "type" : "string",
        "default" : "system-path",
        "markdownDescription": "Where to find the Apache Camel K binary to use",
        "enum" : [ "system-path", "user-specified-path", "tooling-managed"],
        "enumDescriptions": [
            "System path includes the version of the Apache Camel K binary to use",
            "Path to Apache Camel K binary is user-specified in the X setting",
            "Download default version or use user-specified version in the X setting"
        ]
    }

camel-tooling / vscode-camelk

issue with camelk version check and downloading newer version #369

Apache Camel K CLI Installation