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

Marathon provider #392

Merged
merged 20 commits into from
Dec 16, 2015
Merged

Marathon provider #392

merged 20 commits into from
Dec 16, 2015

Conversation

kadel
Copy link
Collaborator

@kadel kadel commented Nov 11, 2015

fixes #357

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.05% when pulling a321284 on kadel:marathon-provider into 1c3390e on projectatomic:master.

@@ -165,7 +165,7 @@ def set_arguments(self):
parser_run.add_argument(
"--provider",
dest="cli_provider",
choices=['docker', 'kubernetes', 'openshift'],
choices=['docker', 'kubernetes', 'openshift', 'marathon'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we replace these list with a constant rather than having to update them on 2 places (run and stop)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, I will do that in another PR.

@cdrage
Copy link
Member

cdrage commented Nov 11, 2015

Awesome work 👍

I understand it's still a WIP, but before we merge after GA / Beta-3 I would like to see a few test examples covered in tests/units/cli/test_cli.py

@rtnpro
Copy link
Contributor

rtnpro commented Nov 12, 2015

👍 LGTM

@@ -1,3 +1,4 @@
anymarkup==0.5.0
lockfile==0.10.2
jsonpointer==1.10.0
requests==2.8.1
Copy link
Member

Choose a reason for hiding this comment

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

To note: We also changed the way that we pull requirements, so "requests" will need to be pulled from yum/apt-get/epel

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.02% when pulling dacf744 on kadel:marathon-provider into 1c3390e on projectatomic:master.

@landscape-bot
Copy link

Code Health
Repository health decreased by 1% when pulling 91826e9 on kadel:marathon-provider into 1c3390e on projectatomic:master.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.02% when pulling 764b8f1 on kadel:marathon-provider into 35952e9 on projectatomic:master.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling 654fdb8 on kadel:marathon-provider into 35952e9 on projectatomic:master.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling f8a82b0 on kadel:marathon-provider into 35952e9 on projectatomic:master.

@landscape-bot
Copy link

Code Health
Repository health increased by 0.67% when pulling b79048d on kadel:marathon-provider into 9914fe7 on projectatomic:master.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling f8a82b0 on kadel:marathon-provider into e35080e on projectatomic:master.

@kadel
Copy link
Collaborator Author

kadel commented Dec 1, 2015

what do you think @cdrage? Is it ready to be merged?

@kadel kadel changed the title [WIP] Marathon provider Marathon provider Dec 1, 2015
scheduler.

This provider requires configuration (`providerapi`) to be able to connect to Marathon API. If no `providerapi` is specified it will use `http://localhost:8080` as
Marathon API url.
Copy link
Member

Choose a reason for hiding this comment

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

Need docs on this, how do we pass providerapi when it's in constants.py? We can provide this in answers.conf yes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, you have to put it in answers.conf
something like this:

providerapi=http://10.1.2.2:8080

@kadel
Copy link
Collaborator Author

kadel commented Dec 16, 2015

On one hand you are right about identifying with Mesos, but on other hand if we rename entire thing to Mesos it would be technically incorrect :-(
I will at least try mention Mesos more in docs.

@kadel
Copy link
Collaborator Author

kadel commented Dec 16, 2015

About auth: Marathon alone doesn't have any authentication. It is usually run in closed environment, or some people put reverse proxy (Nginx) in front of Marathon a do HTTP Basic Auth there.
If this is case, it should be enough to set providerapi to something like this http://user:[email protected]:8080 and it should work ;-)

@dharmit
Copy link
Contributor

dharmit commented Dec 16, 2015

Adding a bit to what @kadel already said regarding identifying with Mesos. Marathon is just one framework. You could very well use Aurora or K8s on top of Mesos to get long running jobs done. But Marathon is the most popular today. Agree about adding Mesos in the docs.

@kadel
Copy link
Collaborator Author

kadel commented Dec 16, 2015

@cdrage @dustymabe I've just opened PR #463 that moves _make_request to Utils.
When it is merged I'll update this PR.

kadel and others added 20 commits December 16, 2015 15:33
List of providers was referenced multiple times in `cli/main.py`. Moved
it to a single place under `atomicapp/constants.py` to avoid updating a
change at two places
Made changes to comply with pep8
when deploying composite apps artifacts from others apps gets added to
this array and they are deployed again
test case artifact - port has to be int
remove ununsed sys import
@cdrage
Copy link
Member

cdrage commented Dec 16, 2015

So make_rest_request has been added to utils and docs have been updated.

Am I missing anything or is this close to be merged?

@dustymabe
Copy link
Contributor

LGTM.

@cdrage
Copy link
Member

cdrage commented Dec 16, 2015

I did one last quick check and it seems everything is in order!

✨ Congrats @kadel @dharmit !! :D Thanks for the hard work to getting this merged! ✨

cdrage added a commit that referenced this pull request Dec 16, 2015
@cdrage cdrage merged commit e55ef71 into projectatomic:master Dec 16, 2015
@kadel kadel deleted the marathon-provider branch December 17, 2015 08:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mesos provider?
7 participants