spec icon indicating copy to clipboard operation
spec copied to clipboard

Installing a dev container `feature` more than once in a `devcontainer.json`

Open joshspicer opened this issue 3 years ago • 19 comments

The current dev container features specification proposal is based on an array-like syntax for the 'features' attribute, and thus allows for the invocation of a single feature >1 with different options.

An execution goal stated in the spec is that

"It should be possible to execute a feature multiple times with different parameters."

We are currently discussing how best to tackle feature installation order (#43), with the discussion leaning toward proceeding without use of an array, meaning that there is a need to explore other mechanisms for invoking a feature to install >1 times.

Proposal

The propose extending the value to include an array of objects.

{
    "image": "azcr.io/joshspicer/my-docker-image",
    "features": {
        "devcontainers/features/dotnet": [
            {
              "version": 3.1,
              "runtimeOnly": true,
              "installUsingApt": true
            },
            {
              "version": 6.0,
              "runtimeOnly": false,
              "installUsingApt": false
            }
        ]
    }
}

This devcontainer.json will invoke the devcontainers/features/dotnet feature twice with two sets of options in order (first with the 3.1 set of arguments, and then 6.0 set of arguments).

joshspicer avatar Jun 17 '22 17:06 joshspicer

This seems like a reasonable proposal to me and meets some of the things @edgonmsft has highlighted - @chrmarti ?

Chuxel avatar Jun 17 '22 20:06 Chuxel

Another option is to extend feature options, such that, they can capture arrays and objects of values. That would allow features to capture feature-global options, the list of versions and version-specific options depending on the tools they install.

chrmarti avatar Jun 27 '22 15:06 chrmarti

My preference is to keep it as simple as possible and not overcomplicate things. I like the proposed array syntax with the full options object inside if we are sticking with an object at the top level for the set of features.

Note that this requires using the same version of the feature for each execution (E.g. devcontainers/features/node@2 would be used for both executions). Do we need to support installing with one feature version with one set of options and another feature version with another set of options? Does the current spec allow specifying the same feature with different versions in the key with the current object syntax, such as the following?

{
    "features": {
        "devcontainers/features/node@1": {
            "version": "14"
        },
        "devcontainers/features/node@2": {
            "version": "16",
            "someNewOptionInV2": "..."
        }
    }
}

jkeech avatar Jun 27 '22 15:06 jkeech

@chrmarti - are you suggesting features that look like this?

"features": {
   "devcontainers/features/[email protected]" : {
       "version": [16, 14, latest]
   }

or equivalently with the shorthand that implies "version"

"features": {
   "devcontainers/features/[email protected]" : [16, 14, latest]
}

How would a user modify >1 option for a given feature? I could imagine wanting a user to install a feature >1 time that uses any permutation of options.

joshspicer avatar Jun 27 '22 17:06 joshspicer

Does the current spec allow specifying the same feature with different versions in the key with the current object syntax, such as the following?

Yes, given the object syntax and these are two unique keys, I think we should (and would have to) support a declaration like this.

joshspicer avatar Jun 27 '22 17:06 joshspicer

I also the array syntax so that the whole options object can be modified for each run.

The version question is an interesting one. The object syntax does allow us to use different versions since they would have different ids, although ordering will probably get interesting and the user would probably need to override it.

edgonmsft avatar Jun 27 '22 17:06 edgonmsft

For the dotnet feature, I can imagine users wanting to install dotnet twice with the following options.

These are the existing options today, and two plausible ways a user could configure several installations of the feature.

{
  "version": 3.1,
  "runtimeOnly": true,
  "installUsingApt": true
}
{
  "version": 6.0,
  "runtimeOnly": false,
  "installUsingApt": false
}

image

joshspicer avatar Jun 27 '22 17:06 joshspicer

@joshspicer @edgonmsft @jkeech @chrmarti

Yeah, to that point - Personally I think we're talking about two separate proposals here.

  1. I think that we will need an array of strings option type. Right now if we added the ability to install a list of packages for a feature like homebrew, you'd need to do a space separated list which isn't great. So an array of strings is a reasonable type. It's also an example I wouldn't want to use this proposal for.
  2. This proposal is going beyond those basic needs and trying to meet the need of varying sets of configuration options for a given feature. You can't resolve that with just an array option type. Something along these lines will likely be needed as well. That is what Josh is showing in https://github.com/devcontainers/spec/issues/44#issuecomment-1167683951.

With that in mind, I'd propose we not discuss this as an "or", but rather whether we think (2) this is needed now. We clearly can see the (1) is needed at a minimum to cover cases we know about. An alternative is also that we implement both at once.

Chuxel avatar Jun 27 '22 17:06 Chuxel

@Chuxel I agree we may be getting side-tracked. I added a new issue to address (1) in your comment: https://github.com/devcontainers/spec/issues/57

I want this proposal to focus on (2), particularly for the reason I highlighted in this comment. This is the first example I could come up with - I think this will be a more generic problem, and I think the syntax I propose is a good solution to that problem.

joshspicer avatar Jun 27 '22 18:06 joshspicer

This is the current interface for the python feature

image

joshspicer avatar Jun 27 '22 18:06 joshspicer

If we allow for more flexibility in the options, features can handle both: multiple versions and more advanced options. Always having an object for a feature also seems to keep the schema simpler. (Not sure if the thinking was to loop over the feature's install.sh, but in general it will be more flexible and potentially less error-prone to let the feature decide how to best handle multiple versions.) The object also allows for global options, e.g., install_jupyterlab might only make sense to have once.

{
	"features": {
		"devcontainers/features/[email protected]": {
			"versions": [
				{
					"version": "3.5",
					"optimize": true
				},
				{
					"version": "3.8",
					"install_python_tools": false
				}
			],
			"defaultVersion": "3.5",
			"install_jupyterlab": true
		}
	}
}

I think we should disallow having the same feature in multiple (feature) versions. That will make it easier for feature authors because they do not need to consider that case.

chrmarti avatar Jun 28 '22 13:06 chrmarti

@chrmarti how would complex objects get passed in and parsed by the install script? I think it will end up being more complicated for both end users and feature authors to deal with complex option types and instead that we should support executing a feature multiple times by looping on the outside with different sets of simple options.

jkeech avatar Jun 28 '22 23:06 jkeech

This seems simple for the end-user. For the feature we can continue to pass in a flattened list of attributes (possibly omitting the nested ones) and also include a variable with the full options JSON. I expect most features to require only simple options.

chrmarti avatar Jun 29 '22 06:06 chrmarti

We agree:

  • we want to proceed with the proposed array syntax
  • it is our goal that features are idempotent / re-entrant / don't explode if they are executed multiple times and we operate under this default.
  • [ ] a feature should be able to declare in its manifest if it is safe to be executed multiple times or not. The default is that it is safe to be executed multiple times, but a feature should have a way to indicate the contrary.
  • [ ] when running the "Add Development Container Configuration Files..." command, the features already installed in the base image should be indicated in the quick pick and potentially not suggested via ctrl+space.
  • [ ] the feature test command could ensure features don't explode if they are executed multiple times.

alexdima avatar Jul 18 '22 14:07 alexdima

Meanwhile, is there any workaround for something like this?

{
    "features": {
		"ghcr.io/devcontainers-contrib/features/asdf-package:1": {
			"plugin": "aws-vault"
		}
       		"ghcr.io/devcontainers-contrib/features/asdf-package:1": {
			"plugin": "consul"
		}
    }
}

Alegrowin avatar Apr 10 '23 16:04 Alegrowin

@Alegrowin There's no workaround within the spec today to install the same Feature twice. We're exploring changing this soon with the introduction of composite/dependencies, but this exact functionality in on our backlog (although I agree it's functionality that would be very nice to have).

Today, a lot of the Features in the wild wouldn't behave nicely if installed twice (either short-circuit early or overwrite existing installs). I personally don't want to introduce a solution the right guidance in place for authors to make the behavior predictable.

For this example, I think a Feature that proxies to another plugin manager should naturally accept >1 plugin. For example, I think it would be nice if upstream the feature worked like so:

{
    "features": {
		"ghcr.io/devcontainers-contrib/features/asdf-package:1": {
			"plugin": "aws-vault,consul"
		}
    }
}

With Feature dependencies though, without rigid rules around options we'd still need to support the case you're illustrating above (installing the Feature twice). And I also understand that someone may want to use a Feature as-is, even if it's a bit ugly.

Thanks for putting this back on our radar as useful to you. Taking this feedback back to our team.

joshspicer avatar Apr 11 '23 23:04 joshspicer