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

You should be able to provide a default provider within Nulecule #378

Closed
cdrage opened this issue Nov 6, 2015 · 22 comments
Closed

You should be able to provide a default provider within Nulecule #378

cdrage opened this issue Nov 6, 2015 · 22 comments
Assignees
Milestone

Comments

@cdrage
Copy link
Member

cdrage commented Nov 6, 2015

An issue that we're facing is that there is no way to assign a default provider outside of the answers.conf.

If an Atomic App is packaged with only a Docker provider, by running atomic run cdrage/testimage the container by default will try to run k8s if no answers.conf is provided.

I propose we add (somewhere) to the Nulecule graph a default provider.

metadata:
  name: Hello Apache App
  appversion: 0.0.1
  description: Atomic app for deploying a really basic Apache HTTP server
graph:
  - name: helloapache-app
  - defaultprovider: docker
    params:
      - name: image
        description: The webserver image
        default: centos/httpd
      - name: hostport
        description: The host TCP port as the external endpoint
        default: 80
    artifacts:
      docker:
        - file://artifacts/docker/hello-apache-pod_run
      kubernetes:
        - file://artifacts/kubernetes/hello-apache-pod.json
@dustymabe
Copy link
Contributor

seems reasonable. @vpavlin @goern ^^

@vpavlin
Copy link
Contributor

vpavlin commented Nov 7, 2015

IMO this should be added as a top level key - it is a "whole app" configuration. So something like:

---
specversion: 0.0.2
id: guestbook-app
defaultprovider: docker

metadata:
  name: Guestbook Application
  appversion: 0.0.1
  description: Atomic app for deploying the guestbook Python app
graph:

Or we just create another section like metadata

---
specversion: 0.0.2
id: guestbook-app

config:
  defaultprovider: docker

metadata:
  name: Guestbook Application
  appversion: 0.0.1
  description: Atomic app for deploying the guestbook Python app
graph:

Also, this is probably a spec change rather then just extension for Atomic App, so it should be a Nulecule repo issue:)

And last thing - provider choice is mostly on the user who deploys the app, not on the developer. If the app contains only a docker provider metadata, we should be able to figure this out (that's one of the purposes of the refactoring done by @rtnpro!) and change the behaviour accordingly - i.e. don't blindly try to talk to kubernetes if the app does not support it.

If we agree that the paragraph above is what we are trying to solve here, then you have my +1 to work on it and fix it (and then the issue obviously belongs here). Otherwise, I'd like to hear more reasons to why the defaultprovider is needed in Nulecule file.

@cdrage
Copy link
Member Author

cdrage commented Nov 7, 2015

@vpavlin I totally agree in regards to the provider choice being able to select a default. If there is only one provider it should default to it, or if there are two providers that aren't k8s, then it should default to at least one of them.

@vpavlin
Copy link
Contributor

vpavlin commented Nov 9, 2015

Can we also try to figure out which providers are available in the user's environment rather than hardcode this into an app?

@aweiteka
Copy link
Contributor

aweiteka commented Nov 9, 2015

I agree this should be a global config, not in the graph. I would suggest it land in the metadata section, not as a first level item.

@vpavlin we have a start towards auto-detection in #346

@aweiteka
Copy link
Contributor

From an end-user perspective I think this scenario validates this as long as we can override the default via answers/cli opt.

  1. I have a system that supports multiple providers, say kubernetes and Docker. I'm trying out a new application to play with it. I want the lowest barrier to get it running (hello, world). The quickstart docs suggest I can "just run it." I will rely on the default provider that's selected for this application. In this case the developer chose Docker as the default provider.
  2. I have played with the application and now want to deploy it on a stage (pre-production) system. I have planned the deployment (storage requirements, config, etc) using documentation. I know what provider I want, this time kubernetes. I want to select which provider, overriding the default.

@kadel
Copy link
Collaborator

kadel commented Nov 20, 2015

How about if we just say that first provider in io.projectatomic.nulecule.providers label is default provider? This way we don't need another configuration option.

@vpavlin
Copy link
Contributor

vpavlin commented Nov 20, 2015

@kadel I think this splits metadata into 2 locations, which doesn't sound right to me. Also there have been some requests on being able to use Nulecule as a tar ball instead of Docker image and if somebody decides to do that, he would lose the ability to specify default provider

@rtnpro
Copy link
Contributor

rtnpro commented Nov 25, 2015

@vpavlin

we don't need to add another key, as in

defaultprovider: docker

We just need to update Nulecule spec to support params in the root level, to incorporate params in general section in the Nulecule spec file.

@cdrage
Copy link
Member Author

cdrage commented Nov 25, 2015

@rtnpro

Could you post in projectatomic/nulecule#184 about your proposal?

@dustymabe dustymabe added this to the CDK 2 GA milestone Nov 25, 2015
@cdrage
Copy link
Member Author

cdrage commented Dec 7, 2015

timeout... ping @rtnpro

@vpavlin
Copy link
Contributor

vpavlin commented Dec 8, 2015

@rtnpro You are right

@rtnpro
Copy link
Contributor

rtnpro commented Dec 22, 2015

@cdrage Global params have been supported in atomicapp from a long time. We just need to update Nulecule spec docs.

@rtnpro
Copy link
Contributor

rtnpro commented Dec 22, 2015

@dustymabe, @vpavlin, do we have a go for updating Nulecule spec on this?

@rtnpro rtnpro self-assigned this Dec 22, 2015
@rtnpro
Copy link
Contributor

rtnpro commented Jan 5, 2016

@cdrage
Copy link
Member Author

cdrage commented Jan 5, 2016

Hmm, so if the global params PR are implemented, I'm assuming this will work just by passing defaultprovider correct?

Just need to update the spec and change a tiny bit of code and we're golden!

@aweiteka
Copy link
Contributor

aweiteka commented Jan 5, 2016

Per previous comments I still recommend we make this an optional extension of the metadata. No spec change is needed. Ideally this is a fallback directive. atomicapp should intelligently determine the default based on capability and an ordered list. If nothing specified, try these providers in order if available.

@dustymabe
Copy link
Contributor

So from aarons comment if we just use the metadata section then a spec file would look like this:

metadata:
  name: Hello Apache App
  appversion: 0.0.1
  description: Atomic app for deploying a really basic Apache HTTP server
  defaultprovider: docker
...

That seems reasonable and wouldn't require an update to the spec. @rtnpro what would an example nulecule file look like if we implemented this in global params?

@rtnpro
Copy link
Contributor

rtnpro commented Jan 11, 2016

@dustymabe the above looks much cleaner compared to

params:
    - name: defaultprovider
      description: Default provider for your Nulecule app
      default: kubernetes
    ...

@vpavlin
Copy link
Contributor

vpavlin commented Jan 11, 2016

No, it does not look cleaner:) It looks like adding another layer of indirection to params

metadata key -> global params -> component params -> answers

My preference is using global params - but I wouldn't use defaultprovider, but simply provider so that it's easily changeable in answers and does not add any code

@rtnpro
Copy link
Contributor

rtnpro commented Jan 11, 2016

I think we should use a poll here: http://poll.gitrun.com/ :)

@dustymabe
Copy link
Contributor

@vpavlin - With your suggestion of using global params and using provider vs defaultprovider, I think that could be a reasonable solution as well. Documenting that you can do that would be the only problem, but I think it is one we could overcome easily by including it in our examples.

@rtnpro do you think using global params as he suggested is reasonable?

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

No branches or pull requests

6 participants