Re: Review Request 44745: Allow for a pure docker executor.

[John Sirois]

On March 13, 2016, 6:04 a.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.
> 
> Bill Farner wrote:
> FWIW i don't think it complicates or even diverges from that ticket.  In 
> my opinion it's yet to be seen whether it's feasible to use the same client 
> for a custom executor (at least, without a decent amount of modularization 
> work).  At the very least, that effort has lost momentum and we shouldn't 
> block progress for it.
> 
> Stephan Erb wrote:
> I mostly brought it up because the ticket also repeatedly mentions the 
> default Mesos command executor. Supporting this one does not sound to 
> different from supporting Docker without Thermos. It would also need similar 
> logic at the UI layer to allow use the Mesos sanbox UI instead of the Thermos 
> one.
> 
> I agree that we should not block progress here. I justed wanted to make 
> sure we are not rushing things (i.e., there isn't even a jira ticket right 
> now).
> 
> Maxim Khutornenko wrote:
> +1 to Stephan's concerns. The schema changes in this patch don't 
> necessarily convey enough meaning to paint a clear picture of where this 
> effort leads us. FWICT, nothing in the Task aside from resources is 
> applicable to the Docker case and it feels quite hacky to onboard a new 
> executor case this way.
> 
> > In my opinion it's yet to be seen whether it's feasible to use the same 
> client for a custom executor (at least, without a decent amount of 
> modularization work).
> 
> Bill, would you mind clarifying what this means? Are you expecting this 
> to be a purely experimental (POC) effort or is there a solid production 
> quality future here? If it's the former, would it be more appropriate to have 
> this effort baked in a private branch to avoid possibly unnecessary code 
> churn and review cycles?
> 
> Bill Farner wrote:
> | it feels quite hacky to onboard a new executor case this way
> 
> Suggestions solicited!  Just please don't forget that the intent is to 
> offer real, immediate value - the docker support in Aurora today is quite 
> crippled, and this will address the biggest shortcomings (entrypoints, and 
> zero required deps in images).
> 
> | FWICT, nothing in the Task aside from resources is applicable to the 
> Docker case
> 
> This is a good point.  Perhaps we should create a separate struct and 
> field in `Job` for this case?
> 
> | Bill, would you mind clarifying what this means?
> 
> What i mean is that the client, DSL, and executor all have relatively 
> high coupling.  Adding custom executor support in the client will require 
> non-trivial effort to break that coupling.  I would like to avoid blocking 
> this feature on that goal.
> 
> | Are you expecting this to be a purely experimental (POC) effort or is 
> there a solid production quality future here?
> 
> That is very much dependent on the underlying support in mesos.  Today, i 
> see it as the best support for docker containers in mesos.  It's been 
> available for some time, and the work here is entirely plumbing to enable it 
> in Aurora.
> 
> | would it be more appropriate to have this effort baked in a private 
> branch to avoid possibly unnecessary code churn and review cycles?
> 
> I don't foresee enough churn to warrant that.
> 
> John Sirois wrote:
> Noting that I'm backing off this change until sentiment settles one way 
> or the other.  If it settles in-favor I'll address both Stephan & Joshua's 
> feedback at that point.
> 
> Stephan Erb wrote:
> I have to backoff out of the discussion here, as I don't have the 
> necessary cycles to participate. A couple of closing notes from my side:
> 
> * I agree with Maxim that giving an empty process list a special meaning 
> feels kind of like a hack.
> * I probably wouldn't have complained about this if it was that way from 
> the beginning...
> * Docker support is still considered experimental, so no decision is cast 
> in stone. We can change stuff without to much hassle.
> * It is great that you are improving the current docker support (even 
> though I am a fanboy of the upcoming unified container support :-)
> 
> Maxim Khutornenko wrote:
> Thanks for explaning Bill. I am fine continuing this effort given the 
> above.
> 
> > This is a good point.  Perhaps we should create a separate struct and 
> field in Job for this case?
> 
> I don't have bandwidth to think about the alternatives at the moment but 
> your suggestion about plugging it higher in the chain (e.g. Job struct) 
> sounds logical.
> 
> Joshua Cohen wrote:
> Could we just add name and resources to the Container struct (if we even 
> need name? I think it's 

Re: Review Request 44745: Allow for a pure docker executor.

[John Sirois]

On March 13, 2016, 6:04 a.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.
> 
> Bill Farner wrote:
> FWIW i don't think it complicates or even diverges from that ticket.  In 
> my opinion it's yet to be seen whether it's feasible to use the same client 
> for a custom executor (at least, without a decent amount of modularization 
> work).  At the very least, that effort has lost momentum and we shouldn't 
> block progress for it.
> 
> Stephan Erb wrote:
> I mostly brought it up because the ticket also repeatedly mentions the 
> default Mesos command executor. Supporting this one does not sound to 
> different from supporting Docker without Thermos. It would also need similar 
> logic at the UI layer to allow use the Mesos sanbox UI instead of the Thermos 
> one.
> 
> I agree that we should not block progress here. I justed wanted to make 
> sure we are not rushing things (i.e., there isn't even a jira ticket right 
> now).
> 
> Maxim Khutornenko wrote:
> +1 to Stephan's concerns. The schema changes in this patch don't 
> necessarily convey enough meaning to paint a clear picture of where this 
> effort leads us. FWICT, nothing in the Task aside from resources is 
> applicable to the Docker case and it feels quite hacky to onboard a new 
> executor case this way.
> 
> > In my opinion it's yet to be seen whether it's feasible to use the same 
> client for a custom executor (at least, without a decent amount of 
> modularization work).
> 
> Bill, would you mind clarifying what this means? Are you expecting this 
> to be a purely experimental (POC) effort or is there a solid production 
> quality future here? If it's the former, would it be more appropriate to have 
> this effort baked in a private branch to avoid possibly unnecessary code 
> churn and review cycles?
> 
> Bill Farner wrote:
> | it feels quite hacky to onboard a new executor case this way
> 
> Suggestions solicited!  Just please don't forget that the intent is to 
> offer real, immediate value - the docker support in Aurora today is quite 
> crippled, and this will address the biggest shortcomings (entrypoints, and 
> zero required deps in images).
> 
> | FWICT, nothing in the Task aside from resources is applicable to the 
> Docker case
> 
> This is a good point.  Perhaps we should create a separate struct and 
> field in `Job` for this case?
> 
> | Bill, would you mind clarifying what this means?
> 
> What i mean is that the client, DSL, and executor all have relatively 
> high coupling.  Adding custom executor support in the client will require 
> non-trivial effort to break that coupling.  I would like to avoid blocking 
> this feature on that goal.
> 
> | Are you expecting this to be a purely experimental (POC) effort or is 
> there a solid production quality future here?
> 
> That is very much dependent on the underlying support in mesos.  Today, i 
> see it as the best support for docker containers in mesos.  It's been 
> available for some time, and the work here is entirely plumbing to enable it 
> in Aurora.
> 
> | would it be more appropriate to have this effort baked in a private 
> branch to avoid possibly unnecessary code churn and review cycles?
> 
> I don't foresee enough churn to warrant that.
> 
> John Sirois wrote:
> Noting that I'm backing off this change until sentiment settles one way 
> or the other.  If it settles in-favor I'll address both Stephan & Joshua's 
> feedback at that point.
> 
> Stephan Erb wrote:
> I have to backoff out of the discussion here, as I don't have the 
> necessary cycles to participate. A couple of closing notes from my side:
> 
> * I agree with Maxim that giving an empty process list a special meaning 
> feels kind of like a hack.
> * I probably wouldn't have complained about this if it was that way from 
> the beginning...
> * Docker support is still considered experimental, so no decision is cast 
> in stone. We can change stuff without to much hassle.
> * It is great that you are improving the current docker support (even 
> though I am a fanboy of the upcoming unified container support :-)
> 
> Maxim Khutornenko wrote:
> Thanks for explaning Bill. I am fine continuing this effort given the 
> above.
> 
> > This is a good point.  Perhaps we should create a separate struct and 
> field in Job for this case?
> 
> I don't have bandwidth to think about the alternatives at the moment but 
> your suggestion about plugging it higher in the chain (e.g. Job struct) 
> sounds logical.
> 
> Joshua Cohen wrote:
> Could we just add name and resources to the Container struct (if we even 
> need name? I think it's 

Re: Review Request 44745: Allow for a pure docker executor.

[John Sirois]

On March 13, 2016, 6:04 a.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.
> 
> Bill Farner wrote:
> FWIW i don't think it complicates or even diverges from that ticket.  In 
> my opinion it's yet to be seen whether it's feasible to use the same client 
> for a custom executor (at least, without a decent amount of modularization 
> work).  At the very least, that effort has lost momentum and we shouldn't 
> block progress for it.
> 
> Stephan Erb wrote:
> I mostly brought it up because the ticket also repeatedly mentions the 
> default Mesos command executor. Supporting this one does not sound to 
> different from supporting Docker without Thermos. It would also need similar 
> logic at the UI layer to allow use the Mesos sanbox UI instead of the Thermos 
> one.
> 
> I agree that we should not block progress here. I justed wanted to make 
> sure we are not rushing things (i.e., there isn't even a jira ticket right 
> now).
> 
> Maxim Khutornenko wrote:
> +1 to Stephan's concerns. The schema changes in this patch don't 
> necessarily convey enough meaning to paint a clear picture of where this 
> effort leads us. FWICT, nothing in the Task aside from resources is 
> applicable to the Docker case and it feels quite hacky to onboard a new 
> executor case this way.
> 
> > In my opinion it's yet to be seen whether it's feasible to use the same 
> client for a custom executor (at least, without a decent amount of 
> modularization work).
> 
> Bill, would you mind clarifying what this means? Are you expecting this 
> to be a purely experimental (POC) effort or is there a solid production 
> quality future here? If it's the former, would it be more appropriate to have 
> this effort baked in a private branch to avoid possibly unnecessary code 
> churn and review cycles?
> 
> Bill Farner wrote:
> | it feels quite hacky to onboard a new executor case this way
> 
> Suggestions solicited!  Just please don't forget that the intent is to 
> offer real, immediate value - the docker support in Aurora today is quite 
> crippled, and this will address the biggest shortcomings (entrypoints, and 
> zero required deps in images).
> 
> | FWICT, nothing in the Task aside from resources is applicable to the 
> Docker case
> 
> This is a good point.  Perhaps we should create a separate struct and 
> field in `Job` for this case?
> 
> | Bill, would you mind clarifying what this means?
> 
> What i mean is that the client, DSL, and executor all have relatively 
> high coupling.  Adding custom executor support in the client will require 
> non-trivial effort to break that coupling.  I would like to avoid blocking 
> this feature on that goal.
> 
> | Are you expecting this to be a purely experimental (POC) effort or is 
> there a solid production quality future here?
> 
> That is very much dependent on the underlying support in mesos.  Today, i 
> see it as the best support for docker containers in mesos.  It's been 
> available for some time, and the work here is entirely plumbing to enable it 
> in Aurora.
> 
> | would it be more appropriate to have this effort baked in a private 
> branch to avoid possibly unnecessary code churn and review cycles?
> 
> I don't foresee enough churn to warrant that.
> 
> John Sirois wrote:
> Noting that I'm backing off this change until sentiment settles one way 
> or the other.  If it settles in-favor I'll address both Stephan & Joshua's 
> feedback at that point.
> 
> Stephan Erb wrote:
> I have to backoff out of the discussion here, as I don't have the 
> necessary cycles to participate. A couple of closing notes from my side:
> 
> * I agree with Maxim that giving an empty process list a special meaning 
> feels kind of like a hack.
> * I probably wouldn't have complained about this if it was that way from 
> the beginning...
> * Docker support is still considered experimental, so no decision is cast 
> in stone. We can change stuff without to much hassle.
> * It is great that you are improving the current docker support (even 
> though I am a fanboy of the upcoming unified container support :-)
> 
> Maxim Khutornenko wrote:
> Thanks for explaning Bill. I am fine continuing this effort given the 
> above.
> 
> > This is a good point.  Perhaps we should create a separate struct and 
> field in Job for this case?
> 
> I don't have bandwidth to think about the alternatives at the moment but 
> your suggestion about plugging it higher in the chain (e.g. Job struct) 
> sounds logical.
> 
> Joshua Cohen wrote:
> Could we just add name and resources to the Container struct (if we even 
> need name? I think it's 

Re: Review Request 44745: Allow for a pure docker executor.

[Joshua Cohen]

On March 13, 2016, 12:04 p.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.
> 
> Bill Farner wrote:
> FWIW i don't think it complicates or even diverges from that ticket.  In 
> my opinion it's yet to be seen whether it's feasible to use the same client 
> for a custom executor (at least, without a decent amount of modularization 
> work).  At the very least, that effort has lost momentum and we shouldn't 
> block progress for it.
> 
> Stephan Erb wrote:
> I mostly brought it up because the ticket also repeatedly mentions the 
> default Mesos command executor. Supporting this one does not sound to 
> different from supporting Docker without Thermos. It would also need similar 
> logic at the UI layer to allow use the Mesos sanbox UI instead of the Thermos 
> one.
> 
> I agree that we should not block progress here. I justed wanted to make 
> sure we are not rushing things (i.e., there isn't even a jira ticket right 
> now).
> 
> Maxim Khutornenko wrote:
> +1 to Stephan's concerns. The schema changes in this patch don't 
> necessarily convey enough meaning to paint a clear picture of where this 
> effort leads us. FWICT, nothing in the Task aside from resources is 
> applicable to the Docker case and it feels quite hacky to onboard a new 
> executor case this way.
> 
> > In my opinion it's yet to be seen whether it's feasible to use the same 
> client for a custom executor (at least, without a decent amount of 
> modularization work).
> 
> Bill, would you mind clarifying what this means? Are you expecting this 
> to be a purely experimental (POC) effort or is there a solid production 
> quality future here? If it's the former, would it be more appropriate to have 
> this effort baked in a private branch to avoid possibly unnecessary code 
> churn and review cycles?
> 
> Bill Farner wrote:
> | it feels quite hacky to onboard a new executor case this way
> 
> Suggestions solicited!  Just please don't forget that the intent is to 
> offer real, immediate value - the docker support in Aurora today is quite 
> crippled, and this will address the biggest shortcomings (entrypoints, and 
> zero required deps in images).
> 
> | FWICT, nothing in the Task aside from resources is applicable to the 
> Docker case
> 
> This is a good point.  Perhaps we should create a separate struct and 
> field in `Job` for this case?
> 
> | Bill, would you mind clarifying what this means?
> 
> What i mean is that the client, DSL, and executor all have relatively 
> high coupling.  Adding custom executor support in the client will require 
> non-trivial effort to break that coupling.  I would like to avoid blocking 
> this feature on that goal.
> 
> | Are you expecting this to be a purely experimental (POC) effort or is 
> there a solid production quality future here?
> 
> That is very much dependent on the underlying support in mesos.  Today, i 
> see it as the best support for docker containers in mesos.  It's been 
> available for some time, and the work here is entirely plumbing to enable it 
> in Aurora.
> 
> | would it be more appropriate to have this effort baked in a private 
> branch to avoid possibly unnecessary code churn and review cycles?
> 
> I don't foresee enough churn to warrant that.
> 
> John Sirois wrote:
> Noting that I'm backing off this change until sentiment settles one way 
> or the other.  If it settles in-favor I'll address both Stephan & Joshua's 
> feedback at that point.
> 
> Stephan Erb wrote:
> I have to backoff out of the discussion here, as I don't have the 
> necessary cycles to participate. A couple of closing notes from my side:
> 
> * I agree with Maxim that giving an empty process list a special meaning 
> feels kind of like a hack.
> * I probably wouldn't have complained about this if it was that way from 
> the beginning...
> * Docker support is still considered experimental, so no decision is cast 
> in stone. We can change stuff without to much hassle.
> * It is great that you are improving the current docker support (even 
> though I am a fanboy of the upcoming unified container support :-)
> 
> Maxim Khutornenko wrote:
> Thanks for explaning Bill. I am fine continuing this effort given the 
> above.
> 
> > This is a good point.  Perhaps we should create a separate struct and 
> field in Job for this case?
> 
> I don't have bandwidth to think about the alternatives at the moment but 
> your suggestion about plugging it higher in the chain (e.g. Job struct) 
> sounds logical.
> 
> Joshua Cohen wrote:
> Could we just add name and resources to the Container struct (if we even 
> need name? I think 

Re: Review Request 44745: Allow for a pure docker executor.

[John Sirois]

On March 13, 2016, 6:04 a.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.
> 
> Bill Farner wrote:
> FWIW i don't think it complicates or even diverges from that ticket.  In 
> my opinion it's yet to be seen whether it's feasible to use the same client 
> for a custom executor (at least, without a decent amount of modularization 
> work).  At the very least, that effort has lost momentum and we shouldn't 
> block progress for it.
> 
> Stephan Erb wrote:
> I mostly brought it up because the ticket also repeatedly mentions the 
> default Mesos command executor. Supporting this one does not sound to 
> different from supporting Docker without Thermos. It would also need similar 
> logic at the UI layer to allow use the Mesos sanbox UI instead of the Thermos 
> one.
> 
> I agree that we should not block progress here. I justed wanted to make 
> sure we are not rushing things (i.e., there isn't even a jira ticket right 
> now).
> 
> Maxim Khutornenko wrote:
> +1 to Stephan's concerns. The schema changes in this patch don't 
> necessarily convey enough meaning to paint a clear picture of where this 
> effort leads us. FWICT, nothing in the Task aside from resources is 
> applicable to the Docker case and it feels quite hacky to onboard a new 
> executor case this way.
> 
> > In my opinion it's yet to be seen whether it's feasible to use the same 
> client for a custom executor (at least, without a decent amount of 
> modularization work).
> 
> Bill, would you mind clarifying what this means? Are you expecting this 
> to be a purely experimental (POC) effort or is there a solid production 
> quality future here? If it's the former, would it be more appropriate to have 
> this effort baked in a private branch to avoid possibly unnecessary code 
> churn and review cycles?
> 
> Bill Farner wrote:
> | it feels quite hacky to onboard a new executor case this way
> 
> Suggestions solicited!  Just please don't forget that the intent is to 
> offer real, immediate value - the docker support in Aurora today is quite 
> crippled, and this will address the biggest shortcomings (entrypoints, and 
> zero required deps in images).
> 
> | FWICT, nothing in the Task aside from resources is applicable to the 
> Docker case
> 
> This is a good point.  Perhaps we should create a separate struct and 
> field in `Job` for this case?
> 
> | Bill, would you mind clarifying what this means?
> 
> What i mean is that the client, DSL, and executor all have relatively 
> high coupling.  Adding custom executor support in the client will require 
> non-trivial effort to break that coupling.  I would like to avoid blocking 
> this feature on that goal.
> 
> | Are you expecting this to be a purely experimental (POC) effort or is 
> there a solid production quality future here?
> 
> That is very much dependent on the underlying support in mesos.  Today, i 
> see it as the best support for docker containers in mesos.  It's been 
> available for some time, and the work here is entirely plumbing to enable it 
> in Aurora.
> 
> | would it be more appropriate to have this effort baked in a private 
> branch to avoid possibly unnecessary code churn and review cycles?
> 
> I don't foresee enough churn to warrant that.
> 
> John Sirois wrote:
> Noting that I'm backing off this change until sentiment settles one way 
> or the other.  If it settles in-favor I'll address both Stephan & Joshua's 
> feedback at that point.
> 
> Stephan Erb wrote:
> I have to backoff out of the discussion here, as I don't have the 
> necessary cycles to participate. A couple of closing notes from my side:
> 
> * I agree with Maxim that giving an empty process list a special meaning 
> feels kind of like a hack.
> * I probably wouldn't have complained about this if it was that way from 
> the beginning...
> * Docker support is still considered experimental, so no decision is cast 
> in stone. We can change stuff without to much hassle.
> * It is great that you are improving the current docker support (even 
> though I am a fanboy of the upcoming unified container support :-)
> 
> Maxim Khutornenko wrote:
> Thanks for explaning Bill. I am fine continuing this effort given the 
> above.
> 
> > This is a good point.  Perhaps we should create a separate struct and 
> field in Job for this case?
> 
> I don't have bandwidth to think about the alternatives at the moment but 
> your suggestion about plugging it higher in the chain (e.g. Job struct) 
> sounds logical.
> 
> Joshua Cohen wrote:
> Could we just add name and resources to the Container struct (if we even 
> need name? I think it's 

Re: Review Request 44745: Allow for a pure docker executor.

[Joshua Cohen]

On March 13, 2016, 12:04 p.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.
> 
> Bill Farner wrote:
> FWIW i don't think it complicates or even diverges from that ticket.  In 
> my opinion it's yet to be seen whether it's feasible to use the same client 
> for a custom executor (at least, without a decent amount of modularization 
> work).  At the very least, that effort has lost momentum and we shouldn't 
> block progress for it.
> 
> Stephan Erb wrote:
> I mostly brought it up because the ticket also repeatedly mentions the 
> default Mesos command executor. Supporting this one does not sound to 
> different from supporting Docker without Thermos. It would also need similar 
> logic at the UI layer to allow use the Mesos sanbox UI instead of the Thermos 
> one.
> 
> I agree that we should not block progress here. I justed wanted to make 
> sure we are not rushing things (i.e., there isn't even a jira ticket right 
> now).
> 
> Maxim Khutornenko wrote:
> +1 to Stephan's concerns. The schema changes in this patch don't 
> necessarily convey enough meaning to paint a clear picture of where this 
> effort leads us. FWICT, nothing in the Task aside from resources is 
> applicable to the Docker case and it feels quite hacky to onboard a new 
> executor case this way.
> 
> > In my opinion it's yet to be seen whether it's feasible to use the same 
> client for a custom executor (at least, without a decent amount of 
> modularization work).
> 
> Bill, would you mind clarifying what this means? Are you expecting this 
> to be a purely experimental (POC) effort or is there a solid production 
> quality future here? If it's the former, would it be more appropriate to have 
> this effort baked in a private branch to avoid possibly unnecessary code 
> churn and review cycles?
> 
> Bill Farner wrote:
> | it feels quite hacky to onboard a new executor case this way
> 
> Suggestions solicited!  Just please don't forget that the intent is to 
> offer real, immediate value - the docker support in Aurora today is quite 
> crippled, and this will address the biggest shortcomings (entrypoints, and 
> zero required deps in images).
> 
> | FWICT, nothing in the Task aside from resources is applicable to the 
> Docker case
> 
> This is a good point.  Perhaps we should create a separate struct and 
> field in `Job` for this case?
> 
> | Bill, would you mind clarifying what this means?
> 
> What i mean is that the client, DSL, and executor all have relatively 
> high coupling.  Adding custom executor support in the client will require 
> non-trivial effort to break that coupling.  I would like to avoid blocking 
> this feature on that goal.
> 
> | Are you expecting this to be a purely experimental (POC) effort or is 
> there a solid production quality future here?
> 
> That is very much dependent on the underlying support in mesos.  Today, i 
> see it as the best support for docker containers in mesos.  It's been 
> available for some time, and the work here is entirely plumbing to enable it 
> in Aurora.
> 
> | would it be more appropriate to have this effort baked in a private 
> branch to avoid possibly unnecessary code churn and review cycles?
> 
> I don't foresee enough churn to warrant that.
> 
> John Sirois wrote:
> Noting that I'm backing off this change until sentiment settles one way 
> or the other.  If it settles in-favor I'll address both Stephan & Joshua's 
> feedback at that point.
> 
> Stephan Erb wrote:
> I have to backoff out of the discussion here, as I don't have the 
> necessary cycles to participate. A couple of closing notes from my side:
> 
> * I agree with Maxim that giving an empty process list a special meaning 
> feels kind of like a hack.
> * I probably wouldn't have complained about this if it was that way from 
> the beginning...
> * Docker support is still considered experimental, so no decision is cast 
> in stone. We can change stuff without to much hassle.
> * It is great that you are improving the current docker support (even 
> though I am a fanboy of the upcoming unified container support :-)
> 
> Maxim Khutornenko wrote:
> Thanks for explaning Bill. I am fine continuing this effort given the 
> above.
> 
> > This is a good point.  Perhaps we should create a separate struct and 
> field in Job for this case?
> 
> I don't have bandwidth to think about the alternatives at the moment but 
> your suggestion about plugging it higher in the chain (e.g. Job struct) 
> sounds logical.
> 
> Joshua Cohen wrote:
> Could we just add name and resources to the Container struct (if we even 
> need name? I think 

Re: Review Request 44745: Allow for a pure docker executor.

[John Sirois]

On March 13, 2016, 6:04 a.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.
> 
> Bill Farner wrote:
> FWIW i don't think it complicates or even diverges from that ticket.  In 
> my opinion it's yet to be seen whether it's feasible to use the same client 
> for a custom executor (at least, without a decent amount of modularization 
> work).  At the very least, that effort has lost momentum and we shouldn't 
> block progress for it.
> 
> Stephan Erb wrote:
> I mostly brought it up because the ticket also repeatedly mentions the 
> default Mesos command executor. Supporting this one does not sound to 
> different from supporting Docker without Thermos. It would also need similar 
> logic at the UI layer to allow use the Mesos sanbox UI instead of the Thermos 
> one.
> 
> I agree that we should not block progress here. I justed wanted to make 
> sure we are not rushing things (i.e., there isn't even a jira ticket right 
> now).
> 
> Maxim Khutornenko wrote:
> +1 to Stephan's concerns. The schema changes in this patch don't 
> necessarily convey enough meaning to paint a clear picture of where this 
> effort leads us. FWICT, nothing in the Task aside from resources is 
> applicable to the Docker case and it feels quite hacky to onboard a new 
> executor case this way.
> 
> > In my opinion it's yet to be seen whether it's feasible to use the same 
> client for a custom executor (at least, without a decent amount of 
> modularization work).
> 
> Bill, would you mind clarifying what this means? Are you expecting this 
> to be a purely experimental (POC) effort or is there a solid production 
> quality future here? If it's the former, would it be more appropriate to have 
> this effort baked in a private branch to avoid possibly unnecessary code 
> churn and review cycles?
> 
> Bill Farner wrote:
> | it feels quite hacky to onboard a new executor case this way
> 
> Suggestions solicited!  Just please don't forget that the intent is to 
> offer real, immediate value - the docker support in Aurora today is quite 
> crippled, and this will address the biggest shortcomings (entrypoints, and 
> zero required deps in images).
> 
> | FWICT, nothing in the Task aside from resources is applicable to the 
> Docker case
> 
> This is a good point.  Perhaps we should create a separate struct and 
> field in `Job` for this case?
> 
> | Bill, would you mind clarifying what this means?
> 
> What i mean is that the client, DSL, and executor all have relatively 
> high coupling.  Adding custom executor support in the client will require 
> non-trivial effort to break that coupling.  I would like to avoid blocking 
> this feature on that goal.
> 
> | Are you expecting this to be a purely experimental (POC) effort or is 
> there a solid production quality future here?
> 
> That is very much dependent on the underlying support in mesos.  Today, i 
> see it as the best support for docker containers in mesos.  It's been 
> available for some time, and the work here is entirely plumbing to enable it 
> in Aurora.
> 
> | would it be more appropriate to have this effort baked in a private 
> branch to avoid possibly unnecessary code churn and review cycles?
> 
> I don't foresee enough churn to warrant that.
> 
> John Sirois wrote:
> Noting that I'm backing off this change until sentiment settles one way 
> or the other.  If it settles in-favor I'll address both Stephan & Joshua's 
> feedback at that point.
> 
> Stephan Erb wrote:
> I have to backoff out of the discussion here, as I don't have the 
> necessary cycles to participate. A couple of closing notes from my side:
> 
> * I agree with Maxim that giving an empty process list a special meaning 
> feels kind of like a hack.
> * I probably wouldn't have complained about this if it was that way from 
> the beginning...
> * Docker support is still considered experimental, so no decision is cast 
> in stone. We can change stuff without to much hassle.
> * It is great that you are improving the current docker support (even 
> though I am a fanboy of the upcoming unified container support :-)
> 
> Maxim Khutornenko wrote:
> Thanks for explaning Bill. I am fine continuing this effort given the 
> above.
> 
> > This is a good point.  Perhaps we should create a separate struct and 
> field in Job for this case?
> 
> I don't have bandwidth to think about the alternatives at the moment but 
> your suggestion about plugging it higher in the chain (e.g. Job struct) 
> sounds logical.
> 
> Joshua Cohen wrote:
> Could we just add name and resources to the Container struct (if we even 
> need name? I think it's 

Re: Review Request 44745: Allow for a pure docker executor.

[Joshua Cohen]

On March 13, 2016, 12:04 p.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.
> 
> Bill Farner wrote:
> FWIW i don't think it complicates or even diverges from that ticket.  In 
> my opinion it's yet to be seen whether it's feasible to use the same client 
> for a custom executor (at least, without a decent amount of modularization 
> work).  At the very least, that effort has lost momentum and we shouldn't 
> block progress for it.
> 
> Stephan Erb wrote:
> I mostly brought it up because the ticket also repeatedly mentions the 
> default Mesos command executor. Supporting this one does not sound to 
> different from supporting Docker without Thermos. It would also need similar 
> logic at the UI layer to allow use the Mesos sanbox UI instead of the Thermos 
> one.
> 
> I agree that we should not block progress here. I justed wanted to make 
> sure we are not rushing things (i.e., there isn't even a jira ticket right 
> now).
> 
> Maxim Khutornenko wrote:
> +1 to Stephan's concerns. The schema changes in this patch don't 
> necessarily convey enough meaning to paint a clear picture of where this 
> effort leads us. FWICT, nothing in the Task aside from resources is 
> applicable to the Docker case and it feels quite hacky to onboard a new 
> executor case this way.
> 
> > In my opinion it's yet to be seen whether it's feasible to use the same 
> client for a custom executor (at least, without a decent amount of 
> modularization work).
> 
> Bill, would you mind clarifying what this means? Are you expecting this 
> to be a purely experimental (POC) effort or is there a solid production 
> quality future here? If it's the former, would it be more appropriate to have 
> this effort baked in a private branch to avoid possibly unnecessary code 
> churn and review cycles?
> 
> Bill Farner wrote:
> | it feels quite hacky to onboard a new executor case this way
> 
> Suggestions solicited!  Just please don't forget that the intent is to 
> offer real, immediate value - the docker support in Aurora today is quite 
> crippled, and this will address the biggest shortcomings (entrypoints, and 
> zero required deps in images).
> 
> | FWICT, nothing in the Task aside from resources is applicable to the 
> Docker case
> 
> This is a good point.  Perhaps we should create a separate struct and 
> field in `Job` for this case?
> 
> | Bill, would you mind clarifying what this means?
> 
> What i mean is that the client, DSL, and executor all have relatively 
> high coupling.  Adding custom executor support in the client will require 
> non-trivial effort to break that coupling.  I would like to avoid blocking 
> this feature on that goal.
> 
> | Are you expecting this to be a purely experimental (POC) effort or is 
> there a solid production quality future here?
> 
> That is very much dependent on the underlying support in mesos.  Today, i 
> see it as the best support for docker containers in mesos.  It's been 
> available for some time, and the work here is entirely plumbing to enable it 
> in Aurora.
> 
> | would it be more appropriate to have this effort baked in a private 
> branch to avoid possibly unnecessary code churn and review cycles?
> 
> I don't foresee enough churn to warrant that.
> 
> John Sirois wrote:
> Noting that I'm backing off this change until sentiment settles one way 
> or the other.  If it settles in-favor I'll address both Stephan & Joshua's 
> feedback at that point.
> 
> Stephan Erb wrote:
> I have to backoff out of the discussion here, as I don't have the 
> necessary cycles to participate. A couple of closing notes from my side:
> 
> * I agree with Maxim that giving an empty process list a special meaning 
> feels kind of like a hack.
> * I probably wouldn't have complained about this if it was that way from 
> the beginning...
> * Docker support is still considered experimental, so no decision is cast 
> in stone. We can change stuff without to much hassle.
> * It is great that you are improving the current docker support (even 
> though I am a fanboy of the upcoming unified container support :-)
> 
> Maxim Khutornenko wrote:
> Thanks for explaning Bill. I am fine continuing this effort given the 
> above.
> 
> > This is a good point.  Perhaps we should create a separate struct and 
> field in Job for this case?
> 
> I don't have bandwidth to think about the alternatives at the moment but 
> your suggestion about plugging it higher in the chain (e.g. Job struct) 
> sounds logical.

Could we just add name and resources to the Container struct (if we even need 
name? I think it's only used by thermos today, 

Re: Review Request 44745: Allow for a pure docker executor.

[Maxim Khutornenko]

On March 13, 2016, 12:04 p.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.
> 
> Bill Farner wrote:
> FWIW i don't think it complicates or even diverges from that ticket.  In 
> my opinion it's yet to be seen whether it's feasible to use the same client 
> for a custom executor (at least, without a decent amount of modularization 
> work).  At the very least, that effort has lost momentum and we shouldn't 
> block progress for it.
> 
> Stephan Erb wrote:
> I mostly brought it up because the ticket also repeatedly mentions the 
> default Mesos command executor. Supporting this one does not sound to 
> different from supporting Docker without Thermos. It would also need similar 
> logic at the UI layer to allow use the Mesos sanbox UI instead of the Thermos 
> one.
> 
> I agree that we should not block progress here. I justed wanted to make 
> sure we are not rushing things (i.e., there isn't even a jira ticket right 
> now).
> 
> Maxim Khutornenko wrote:
> +1 to Stephan's concerns. The schema changes in this patch don't 
> necessarily convey enough meaning to paint a clear picture of where this 
> effort leads us. FWICT, nothing in the Task aside from resources is 
> applicable to the Docker case and it feels quite hacky to onboard a new 
> executor case this way.
> 
> > In my opinion it's yet to be seen whether it's feasible to use the same 
> client for a custom executor (at least, without a decent amount of 
> modularization work).
> 
> Bill, would you mind clarifying what this means? Are you expecting this 
> to be a purely experimental (POC) effort or is there a solid production 
> quality future here? If it's the former, would it be more appropriate to have 
> this effort baked in a private branch to avoid possibly unnecessary code 
> churn and review cycles?
> 
> Bill Farner wrote:
> | it feels quite hacky to onboard a new executor case this way
> 
> Suggestions solicited!  Just please don't forget that the intent is to 
> offer real, immediate value - the docker support in Aurora today is quite 
> crippled, and this will address the biggest shortcomings (entrypoints, and 
> zero required deps in images).
> 
> | FWICT, nothing in the Task aside from resources is applicable to the 
> Docker case
> 
> This is a good point.  Perhaps we should create a separate struct and 
> field in `Job` for this case?
> 
> | Bill, would you mind clarifying what this means?
> 
> What i mean is that the client, DSL, and executor all have relatively 
> high coupling.  Adding custom executor support in the client will require 
> non-trivial effort to break that coupling.  I would like to avoid blocking 
> this feature on that goal.
> 
> | Are you expecting this to be a purely experimental (POC) effort or is 
> there a solid production quality future here?
> 
> That is very much dependent on the underlying support in mesos.  Today, i 
> see it as the best support for docker containers in mesos.  It's been 
> available for some time, and the work here is entirely plumbing to enable it 
> in Aurora.
> 
> | would it be more appropriate to have this effort baked in a private 
> branch to avoid possibly unnecessary code churn and review cycles?
> 
> I don't foresee enough churn to warrant that.
> 
> John Sirois wrote:
> Noting that I'm backing off this change until sentiment settles one way 
> or the other.  If it settles in-favor I'll address both Stephan & Joshua's 
> feedback at that point.
> 
> Stephan Erb wrote:
> I have to backoff out of the discussion here, as I don't have the 
> necessary cycles to participate. A couple of closing notes from my side:
> 
> * I agree with Maxim that giving an empty process list a special meaning 
> feels kind of like a hack.
> * I probably wouldn't have complained about this if it was that way from 
> the beginning...
> * Docker support is still considered experimental, so no decision is cast 
> in stone. We can change stuff without to much hassle.
> * It is great that you are improving the current docker support (even 
> though I am a fanboy of the upcoming unified container support :-)

Thanks for explaning Bill. I am fine continuing this effort given the above.

> This is a good point.  Perhaps we should create a separate struct and field 
> in Job for this case?

I don't have bandwidth to think about the alternatives at the moment but your 
suggestion about plugging it higher in the chain (e.g. Job struct) sounds 
logical.


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44745/#review123314

Re: Review Request 44745: Allow for a pure docker executor.

[Stephan Erb]

On March 13, 2016, 1:04 p.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.
> 
> Bill Farner wrote:
> FWIW i don't think it complicates or even diverges from that ticket.  In 
> my opinion it's yet to be seen whether it's feasible to use the same client 
> for a custom executor (at least, without a decent amount of modularization 
> work).  At the very least, that effort has lost momentum and we shouldn't 
> block progress for it.
> 
> Stephan Erb wrote:
> I mostly brought it up because the ticket also repeatedly mentions the 
> default Mesos command executor. Supporting this one does not sound to 
> different from supporting Docker without Thermos. It would also need similar 
> logic at the UI layer to allow use the Mesos sanbox UI instead of the Thermos 
> one.
> 
> I agree that we should not block progress here. I justed wanted to make 
> sure we are not rushing things (i.e., there isn't even a jira ticket right 
> now).
> 
> Maxim Khutornenko wrote:
> +1 to Stephan's concerns. The schema changes in this patch don't 
> necessarily convey enough meaning to paint a clear picture of where this 
> effort leads us. FWICT, nothing in the Task aside from resources is 
> applicable to the Docker case and it feels quite hacky to onboard a new 
> executor case this way.
> 
> > In my opinion it's yet to be seen whether it's feasible to use the same 
> client for a custom executor (at least, without a decent amount of 
> modularization work).
> 
> Bill, would you mind clarifying what this means? Are you expecting this 
> to be a purely experimental (POC) effort or is there a solid production 
> quality future here? If it's the former, would it be more appropriate to have 
> this effort baked in a private branch to avoid possibly unnecessary code 
> churn and review cycles?
> 
> Bill Farner wrote:
> | it feels quite hacky to onboard a new executor case this way
> 
> Suggestions solicited!  Just please don't forget that the intent is to 
> offer real, immediate value - the docker support in Aurora today is quite 
> crippled, and this will address the biggest shortcomings (entrypoints, and 
> zero required deps in images).
> 
> | FWICT, nothing in the Task aside from resources is applicable to the 
> Docker case
> 
> This is a good point.  Perhaps we should create a separate struct and 
> field in `Job` for this case?
> 
> | Bill, would you mind clarifying what this means?
> 
> What i mean is that the client, DSL, and executor all have relatively 
> high coupling.  Adding custom executor support in the client will require 
> non-trivial effort to break that coupling.  I would like to avoid blocking 
> this feature on that goal.
> 
> | Are you expecting this to be a purely experimental (POC) effort or is 
> there a solid production quality future here?
> 
> That is very much dependent on the underlying support in mesos.  Today, i 
> see it as the best support for docker containers in mesos.  It's been 
> available for some time, and the work here is entirely plumbing to enable it 
> in Aurora.
> 
> | would it be more appropriate to have this effort baked in a private 
> branch to avoid possibly unnecessary code churn and review cycles?
> 
> I don't foresee enough churn to warrant that.
> 
> John Sirois wrote:
> Noting that I'm backing off this change until sentiment settles one way 
> or the other.  If it settles in-favor I'll address both Stephan & Joshua's 
> feedback at that point.

I have to backoff out of the discussion here, as I don't have the necessary 
cycles to participate. A couple of closing notes from my side:

* I agree with Maxim that giving an empty process list a special meaning feels 
kind of like a hack.
* I probably wouldn't have complained about this if it was that way from the 
beginning...
* Docker support is still considered experimental, so no decision is cast in 
stone. We can change stuff without to much hassle.
* It is great that you are improving the current docker support (even though I 
am a fanboy of the upcoming unified container support :-)


- Stephan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44745/#review123314
---


On March 13, 2016, 3:48 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44745/
> ---
> 
> (Updated March 13, 2016, 3:48 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen 

Re: Review Request 44745: Allow for a pure docker executor.

[Maxim Khutornenko]

On March 13, 2016, 12:04 p.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.
> 
> Bill Farner wrote:
> FWIW i don't think it complicates or even diverges from that ticket.  In 
> my opinion it's yet to be seen whether it's feasible to use the same client 
> for a custom executor (at least, without a decent amount of modularization 
> work).  At the very least, that effort has lost momentum and we shouldn't 
> block progress for it.
> 
> Stephan Erb wrote:
> I mostly brought it up because the ticket also repeatedly mentions the 
> default Mesos command executor. Supporting this one does not sound to 
> different from supporting Docker without Thermos. It would also need similar 
> logic at the UI layer to allow use the Mesos sanbox UI instead of the Thermos 
> one.
> 
> I agree that we should not block progress here. I justed wanted to make 
> sure we are not rushing things (i.e., there isn't even a jira ticket right 
> now).

+1 to Stephan's concerns. The schema changes in this patch don't necessarily 
convey enough meaning to paint a clear picture of where this effort leads us. 
FWICT, nothing in the Task aside from resources is applicable to the Docker 
case and it feels quite hacky to onboard a new executor case this way.

> In my opinion it's yet to be seen whether it's feasible to use the same 
> client for a custom executor (at least, without a decent amount of 
> modularization work).

Bill, would you mind clarifying what this means? Are you expecting this to be a 
purely experimental (POC) effort or is there a solid production quality future 
here? If it's the former, would it be more appropriate to have this effort 
baked in a private branch to avoid possibly unnecessary code churn and review 
cycles?


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44745/#review123314
---


On March 13, 2016, 2:48 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44745/
> ---
> 
> (Updated March 13, 2016, 2:48 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This allows for a job config with no processes defined IFF there is also
> a Docker container defined.  In this case it is assumed that the process
> to run is the Docker container's ENTRYPOINT via the Mesos Docker
> containerizer.
> 
>  src/main/python/apache/aurora/config/__init__.py| 26 
> +++---
>  src/main/python/apache/aurora/config/thrift.py  |  9 +
>  src/test/python/apache/aurora/client/test_config.py | 41 
> +++--
>  src/test/python/apache/aurora/config/test_thrift.py |  5 +
>  4 files changed, 68 insertions(+), 13 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/config/__init__.py 
> 65923be1cb8b88139b8eab0ac5b75428972d3cb1 
>   src/main/python/apache/aurora/config/thrift.py 
> be0cd68674a71bd4baadf276f40a4bc0223ce4be 
>   src/main/python/apache/thermos/config/schema_base.py 
> a6768e67189b0560afef844d6b269bed8ada5f2f 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 88292d3c4423c0555088a0adaee3c0e62ed0567e 
> 
> Diff: https://reviews.apache.org/r/44745/diff/
> 
> 
> Testing
> ---
> 
> Locally green `./build-support/jenkins/build.sh`
> 
> I also patched in https://reviews.apache.org/r/44685/ which this change
> depends on and was able to run scheduler with
> `-allow_docker_parameters -require_docker_use_executor` and successfully
> run this job:
> ```
> import getpass
> 
> jobs=[
>   Service(
> cluster = 'devcluster',
> role = getpass.getuser(),
> environment = 'test',
> name = 'http_example_docker_executor',
> contact = '{{role}}@localhost',
> instances = 1,
> task = Task(
>   name = 'http_docker_example',
>   resources = Resources(cpu=0.4, ram=32*MB, disk=64*MB),
>   processes = []
> ),
> container = Container(
>   docker = Docker(
> image = 'http_example',
> parameters = [
>   Parameter(name = 'env', value = 'HTTP_PORT='),
>   Parameter(name = 'expose', value = ''),
>   Parameter(name = 'publish', value = ':/tcp'),
> ],
>   ),
> ),
>   )
> ]
> ```
> 
> Using the image created with
> `docker build -t http_example 

Re: Review Request 44745: Allow for a pure docker executor.

[John Sirois]

> On March 14, 2016, 9:31 a.m., Joshua Cohen wrote:
> > This seems like something we should cover with the end to end tests? Would 
> > you mind adding a test that spins up the scheduler to allow executor-less 
> > tasks, launches a task and then confirms it responds as expected?
> > 
> > Also, if I understand things correctly, the Dockerfile used to build the 
> > image can differ from our standard http_example image in that we shouldn't 
> > require libcurl4-nss-dev, since nothing is using mesos.native. Is that 
> > right?

Sounds good and that's right.  I was holding off additionally on docs to 
explain this alternate mode of operation.  I'll put in that effort as well 
since by your feedback on https://reviews.apache.org/r/44685/ indicates 
willingness to accept this pair of changes.


- John


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44745/#review123425
---


On March 12, 2016, 7:48 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44745/
> ---
> 
> (Updated March 12, 2016, 7:48 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This allows for a job config with no processes defined IFF there is also
> a Docker container defined.  In this case it is assumed that the process
> to run is the Docker container's ENTRYPOINT via the Mesos Docker
> containerizer.
> 
>  src/main/python/apache/aurora/config/__init__.py| 26 
> +++---
>  src/main/python/apache/aurora/config/thrift.py  |  9 +
>  src/test/python/apache/aurora/client/test_config.py | 41 
> +++--
>  src/test/python/apache/aurora/config/test_thrift.py |  5 +
>  4 files changed, 68 insertions(+), 13 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/config/__init__.py 
> 65923be1cb8b88139b8eab0ac5b75428972d3cb1 
>   src/main/python/apache/aurora/config/thrift.py 
> be0cd68674a71bd4baadf276f40a4bc0223ce4be 
>   src/main/python/apache/thermos/config/schema_base.py 
> a6768e67189b0560afef844d6b269bed8ada5f2f 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 88292d3c4423c0555088a0adaee3c0e62ed0567e 
> 
> Diff: https://reviews.apache.org/r/44745/diff/
> 
> 
> Testing
> ---
> 
> Locally green `./build-support/jenkins/build.sh`
> 
> I also patched in https://reviews.apache.org/r/44685/ which this change
> depends on and was able to run scheduler with
> `-allow_docker_parameters -require_docker_use_executor` and successfully
> run this job:
> ```
> import getpass
> 
> jobs=[
>   Service(
> cluster = 'devcluster',
> role = getpass.getuser(),
> environment = 'test',
> name = 'http_example_docker_executor',
> contact = '{{role}}@localhost',
> instances = 1,
> task = Task(
>   name = 'http_docker_example',
>   resources = Resources(cpu=0.4, ram=32*MB, disk=64*MB),
>   processes = []
> ),
> container = Container(
>   docker = Docker(
> image = 'http_example',
> parameters = [
>   Parameter(name = 'env', value = 'HTTP_PORT='),
>   Parameter(name = 'expose', value = ''),
>   Parameter(name = 'publish', value = ':/tcp'),
> ],
>   ),
> ),
>   )
> ]
> ```
> 
> Using the image created with
> `docker build -t http_example src/test/sh/org/apache/aurora/e2e` from:
> ```
> FROM python:2.7
> 
> # mesos.native requires libcurl-nss to initialize MesosExecutorDriver
> RUN apt-get update && apt-get -y install libcurl4-nss-dev
> 
> COPY http_example.py /tmp/
> ENTRYPOINT python /tmp/http_example.py $HTTP_PORT
> ```
> 
> I could connect to http://aurora.local: and get `Hello!` back.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 44745: Allow for a pure docker executor.

[Joshua Cohen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44745/#review123425
---



This seems like something we should cover with the end to end tests? Would you 
mind adding a test that spins up the scheduler to allow executor-less tasks, 
launches a task and then confirms it responds as expected?

Also, if I understand things correctly, the Dockerfile used to build the image 
can differ from our standard http_example image in that we shouldn't require 
libcurl4-nss-dev, since nothing is using mesos.native. Is that right?

- Joshua Cohen


On March 13, 2016, 2:48 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44745/
> ---
> 
> (Updated March 13, 2016, 2:48 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This allows for a job config with no processes defined IFF there is also
> a Docker container defined.  In this case it is assumed that the process
> to run is the Docker container's ENTRYPOINT via the Mesos Docker
> containerizer.
> 
>  src/main/python/apache/aurora/config/__init__.py| 26 
> +++---
>  src/main/python/apache/aurora/config/thrift.py  |  9 +
>  src/test/python/apache/aurora/client/test_config.py | 41 
> +++--
>  src/test/python/apache/aurora/config/test_thrift.py |  5 +
>  4 files changed, 68 insertions(+), 13 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/config/__init__.py 
> 65923be1cb8b88139b8eab0ac5b75428972d3cb1 
>   src/main/python/apache/aurora/config/thrift.py 
> be0cd68674a71bd4baadf276f40a4bc0223ce4be 
>   src/main/python/apache/thermos/config/schema_base.py 
> a6768e67189b0560afef844d6b269bed8ada5f2f 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 88292d3c4423c0555088a0adaee3c0e62ed0567e 
> 
> Diff: https://reviews.apache.org/r/44745/diff/
> 
> 
> Testing
> ---
> 
> Locally green `./build-support/jenkins/build.sh`
> 
> I also patched in https://reviews.apache.org/r/44685/ which this change
> depends on and was able to run scheduler with
> `-allow_docker_parameters -require_docker_use_executor` and successfully
> run this job:
> ```
> import getpass
> 
> jobs=[
>   Service(
> cluster = 'devcluster',
> role = getpass.getuser(),
> environment = 'test',
> name = 'http_example_docker_executor',
> contact = '{{role}}@localhost',
> instances = 1,
> task = Task(
>   name = 'http_docker_example',
>   resources = Resources(cpu=0.4, ram=32*MB, disk=64*MB),
>   processes = []
> ),
> container = Container(
>   docker = Docker(
> image = 'http_example',
> parameters = [
>   Parameter(name = 'env', value = 'HTTP_PORT='),
>   Parameter(name = 'expose', value = ''),
>   Parameter(name = 'publish', value = ':/tcp'),
> ],
>   ),
> ),
>   )
> ]
> ```
> 
> Using the image created with
> `docker build -t http_example src/test/sh/org/apache/aurora/e2e` from:
> ```
> FROM python:2.7
> 
> # mesos.native requires libcurl-nss to initialize MesosExecutorDriver
> RUN apt-get update && apt-get -y install libcurl4-nss-dev
> 
> COPY http_example.py /tmp/
> ENTRYPOINT python /tmp/http_example.py $HTTP_PORT
> ```
> 
> I could connect to http://aurora.local: and get `Hello!` back.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 44745: Allow for a pure docker executor.

[Bill Farner]

On March 13, 2016, 5:04 a.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.

FWIW i don't think it complicates or even diverges from that ticket.  In my 
opinion it's yet to be seen whether it's feasible to use the same client for a 
custom executor (at least, without a decent amount of modularization work).  At 
the very least, that effort has lost momentum and we shouldn't block progress 
for it.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44745/#review123314
---


On March 12, 2016, 6:48 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44745/
> ---
> 
> (Updated March 12, 2016, 6:48 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This allows for a job config with no processes defined IFF there is also
> a Docker container defined.  In this case it is assumed that the process
> to run is the Docker container's ENTRYPOINT via the Mesos Docker
> containerizer.
> 
>  src/main/python/apache/aurora/config/__init__.py| 26 
> +++---
>  src/main/python/apache/aurora/config/thrift.py  |  9 +
>  src/test/python/apache/aurora/client/test_config.py | 41 
> +++--
>  src/test/python/apache/aurora/config/test_thrift.py |  5 +
>  4 files changed, 68 insertions(+), 13 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/config/__init__.py 
> 65923be1cb8b88139b8eab0ac5b75428972d3cb1 
>   src/main/python/apache/aurora/config/thrift.py 
> be0cd68674a71bd4baadf276f40a4bc0223ce4be 
>   src/main/python/apache/thermos/config/schema_base.py 
> a6768e67189b0560afef844d6b269bed8ada5f2f 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 88292d3c4423c0555088a0adaee3c0e62ed0567e 
> 
> Diff: https://reviews.apache.org/r/44745/diff/
> 
> 
> Testing
> ---
> 
> Locally green `./build-support/jenkins/build.sh`
> 
> I also patched in https://reviews.apache.org/r/44685/ which this change
> depends on and was able to run scheduler with
> `-allow_docker_parameters -require_docker_use_executor` and successfully
> run this job:
> ```
> import getpass
> 
> jobs=[
>   Service(
> cluster = 'devcluster',
> role = getpass.getuser(),
> environment = 'test',
> name = 'http_example_docker_executor',
> contact = '{{role}}@localhost',
> instances = 1,
> task = Task(
>   name = 'http_docker_example',
>   resources = Resources(cpu=0.4, ram=32*MB, disk=64*MB),
>   processes = []
> ),
> container = Container(
>   docker = Docker(
> image = 'http_example',
> parameters = [
>   Parameter(name = 'env', value = 'HTTP_PORT='),
>   Parameter(name = 'expose', value = ''),
>   Parameter(name = 'publish', value = ':/tcp'),
> ],
>   ),
> ),
>   )
> ]
> ```
> 
> Using the image created with
> `docker build -t http_example src/test/sh/org/apache/aurora/e2e` from:
> ```
> FROM python:2.7
> 
> # mesos.native requires libcurl-nss to initialize MesosExecutorDriver
> RUN apt-get update && apt-get -y install libcurl4-nss-dev
> 
> COPY http_example.py /tmp/
> ENTRYPOINT python /tmp/http_example.py $HTTP_PORT
> ```
> 
> I could connect to http://aurora.local: and get `Hello!` back.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 44745: Allow for a pure docker executor.

[Stephan Erb]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44745/#review123314
---




src/main/python/apache/aurora/config/__init__.py (line 142)


Unnecessary space



src/main/python/apache/aurora/config/thrift.py (line 249)


How about using dependency injection for the `ExecutorConfig`? Having such 
an `if` in a pure translation/serialization function feels like we are 
implementing logic at a wrong layer.



src/main/python/apache/thermos/config/schema_base.py (line 96)


What will happen to the Task name if the process list is empty? Runtime 
exception in Thermos?


While your patch is rather easy, I am not sure it is the best way to move 
forward. It feels like it is crossing streams with 
https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought into 
this might be helpful in the long run.

- Stephan Erb


On March 13, 2016, 3:48 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44745/
> ---
> 
> (Updated March 13, 2016, 3:48 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This allows for a job config with no processes defined IFF there is also
> a Docker container defined.  In this case it is assumed that the process
> to run is the Docker container's ENTRYPOINT via the Mesos Docker
> containerizer.
> 
>  src/main/python/apache/aurora/config/__init__.py| 26 
> +++---
>  src/main/python/apache/aurora/config/thrift.py  |  9 +
>  src/test/python/apache/aurora/client/test_config.py | 41 
> +++--
>  src/test/python/apache/aurora/config/test_thrift.py |  5 +
>  4 files changed, 68 insertions(+), 13 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/config/__init__.py 
> 65923be1cb8b88139b8eab0ac5b75428972d3cb1 
>   src/main/python/apache/aurora/config/thrift.py 
> be0cd68674a71bd4baadf276f40a4bc0223ce4be 
>   src/main/python/apache/thermos/config/schema_base.py 
> a6768e67189b0560afef844d6b269bed8ada5f2f 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 88292d3c4423c0555088a0adaee3c0e62ed0567e 
> 
> Diff: https://reviews.apache.org/r/44745/diff/
> 
> 
> Testing
> ---
> 
> Locally green `./build-support/jenkins/build.sh`
> 
> I also patched in https://reviews.apache.org/r/44685/ which this change
> depends on and was able to run scheduler with
> `-allow_docker_parameters -require_docker_use_executor` and successfully
> run this job:
> ```
> import getpass
> 
> jobs=[
>   Service(
> cluster = 'devcluster',
> role = getpass.getuser(),
> environment = 'test',
> name = 'http_example_docker_executor',
> contact = '{{role}}@localhost',
> instances = 1,
> task = Task(
>   name = 'http_docker_example',
>   resources = Resources(cpu=0.4, ram=32*MB, disk=64*MB),
>   processes = []
> ),
> container = Container(
>   docker = Docker(
> image = 'http_example',
> parameters = [
>   Parameter(name = 'env', value = 'HTTP_PORT='),
>   Parameter(name = 'expose', value = ''),
>   Parameter(name = 'publish', value = ':/tcp'),
> ],
>   ),
> ),
>   )
> ]
> ```
> 
> Using the image created with
> `docker build -t http_example src/test/sh/org/apache/aurora/e2e` from:
> ```
> FROM python:2.7
> 
> # mesos.native requires libcurl-nss to initialize MesosExecutorDriver
> RUN apt-get update && apt-get -y install libcurl4-nss-dev
> 
> COPY http_example.py /tmp/
> ENTRYPOINT python /tmp/http_example.py $HTTP_PORT
> ```
> 
> I could connect to http://aurora.local: and get `Hello!` back.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 44745: Allow for a pure docker executor.

[Aurora ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44745/#review123293
---


Ship it!




Master (de128a1) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On March 13, 2016, 2:48 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44745/
> ---
> 
> (Updated March 13, 2016, 2:48 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This allows for a job config with no processes defined IFF there is also
> a Docker container defined.  In this case it is assumed that the process
> to run is the Docker container's ENTRYPOINT via the Mesos Docker
> containerizer.
> 
>  src/main/python/apache/aurora/config/__init__.py| 26 
> +++---
>  src/main/python/apache/aurora/config/thrift.py  |  9 +
>  src/test/python/apache/aurora/client/test_config.py | 41 
> +++--
>  src/test/python/apache/aurora/config/test_thrift.py |  5 +
>  4 files changed, 68 insertions(+), 13 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/config/__init__.py 
> 65923be1cb8b88139b8eab0ac5b75428972d3cb1 
>   src/main/python/apache/aurora/config/thrift.py 
> be0cd68674a71bd4baadf276f40a4bc0223ce4be 
>   src/main/python/apache/thermos/config/schema_base.py 
> a6768e67189b0560afef844d6b269bed8ada5f2f 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 88292d3c4423c0555088a0adaee3c0e62ed0567e 
> 
> Diff: https://reviews.apache.org/r/44745/diff/
> 
> 
> Testing
> ---
> 
> Locally green `./build-support/jenkins/build.sh`
> 
> I also patched in https://reviews.apache.org/r/44685/ which this change
> depends on and was able to run scheduler with
> `-allow_docker_parameters -require_docker_use_executor` and successfully
> run this job:
> ```
> import getpass
> 
> jobs=[
>   Service(
> cluster = 'devcluster',
> role = getpass.getuser(),
> environment = 'test',
> name = 'http_example_docker_executor',
> contact = '{{role}}@localhost',
> instances = 1,
> task = Task(
>   name = 'http_docker_example',
>   resources = Resources(cpu=0.4, ram=32*MB, disk=64*MB),
>   processes = []
> ),
> container = Container(
>   docker = Docker(
> image = 'http_example',
> parameters = [
>   Parameter(name = 'env', value = 'HTTP_PORT='),
>   Parameter(name = 'expose', value = ''),
>   Parameter(name = 'publish', value = ':/tcp'),
> ],
>   ),
> ),
>   )
> ]
> ```
> 
> Using the image created with
> `docker build -t http_example src/test/sh/org/apache/aurora/e2e` from:
> ```
> FROM python:2.7
> 
> # mesos.native requires libcurl-nss to initialize MesosExecutorDriver
> RUN apt-get update && apt-get -y install libcurl4-nss-dev
> 
> COPY http_example.py /tmp/
> ENTRYPOINT python /tmp/http_example.py $HTTP_PORT
> ```
> 
> I could connect to http://aurora.local: and get `Hello!` back.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 44745: Allow for a pure docker executor.

[John Sirois]

> On March 12, 2016, 1:37 p.m., Bill Farner wrote:
> > LGTM overall, though if possible it would be nice to omit the 
> > `processes=[]` line altogether.

Agreed, and the error was cryptic to boot when omitted previously - fixed.


> On March 12, 2016, 1:37 p.m., Bill Farner wrote:
> > src/main/python/apache/aurora/config/__init__.py, line 169
> > 
> >
> > s/Processes/Process/

Fixed.


- John


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44745/#review123288
---


On March 11, 2016, 6:04 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44745/
> ---
> 
> (Updated March 11, 2016, 6:04 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This allows for a job config with no processes defined IFF there is also
> a Docker container defined.  In this case it is assumed that the process
> to run is the Docker container's ENTRYPOINT via the Mesos Docker
> containerizer.
> 
>  src/main/python/apache/aurora/config/__init__.py| 26 
> +++---
>  src/main/python/apache/aurora/config/thrift.py  |  9 +
>  src/test/python/apache/aurora/client/test_config.py | 41 
> +++--
>  src/test/python/apache/aurora/config/test_thrift.py |  5 +
>  4 files changed, 68 insertions(+), 13 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/config/__init__.py 
> 65923be1cb8b88139b8eab0ac5b75428972d3cb1 
>   src/main/python/apache/aurora/config/thrift.py 
> be0cd68674a71bd4baadf276f40a4bc0223ce4be 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 88292d3c4423c0555088a0adaee3c0e62ed0567e 
> 
> Diff: https://reviews.apache.org/r/44745/diff/
> 
> 
> Testing
> ---
> 
> Locally green `./build-support/jenkins/build.sh`
> 
> I also patched in https://reviews.apache.org/r/44685/ which this change
> depends on and was able to run scheduler with
> `-allow_docker_parameters -require_docker_use_executor` and successfully
> run this job:
> ```
> import getpass
> 
> jobs=[
>   Service(
> cluster = 'devcluster',
> role = getpass.getuser(),
> environment = 'test',
> name = 'http_example_docker_executor',
> contact = '{{role}}@localhost',
> instances = 1,
> task = Task(
>   name = 'http_docker_example',
>   resources = Resources(cpu=0.4, ram=32*MB, disk=64*MB),
>   processes = []
> ),
> container = Container(
>   docker = Docker(
> image = 'http_example',
> parameters = [
>   Parameter(name = 'env', value = 'HTTP_PORT='),
>   Parameter(name = 'expose', value = ''),
>   Parameter(name = 'publish', value = ':/tcp'),
> ],
>   ),
> ),
>   )
> ]
> ```
> 
> Using the image created with
> `docker build -t http_example src/test/sh/org/apache/aurora/e2e` from:
> ```
> FROM python:2.7
> 
> # mesos.native requires libcurl-nss to initialize MesosExecutorDriver
> RUN apt-get update && apt-get -y install libcurl4-nss-dev
> 
> COPY http_example.py /tmp/
> ENTRYPOINT python /tmp/http_example.py $HTTP_PORT
> ```
> 
> I could connect to http://aurora.local: and get `Hello!` back.
> 
> 
> Thanks,
> 
> John Sirois
> 
>