Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

[DC/OS] Add support for DCOS 1.9 EA#360

Merged
JackQuincy merged 17 commits intoAzure:masterfrom
wbuchwalter:dcos-19ea
Mar 28, 2017
Merged

[DC/OS] Add support for DCOS 1.9 EA#360
JackQuincy merged 17 commits intoAzure:masterfrom
wbuchwalter:dcos-19ea

Conversation

@wbuchwalter
Copy link
Contributor

@wbuchwalter wbuchwalter commented Mar 3, 2017

Addresses issue #359.


This change is Reviewable

@msftclas
Copy link

msftclas commented Mar 3, 2017

@wbuchwalter,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@acs-bot
Copy link

acs-bot commented Mar 3, 2017

Can one of the admins verify this patch?

@wbuchwalter
Copy link
Contributor Author

I have added some documentation to help for future releases, would be great if someone that already went through the process could review it (and tell me if there is an easier way 😃)

@acs-bot
Copy link

acs-bot commented Mar 7, 2017

Can one of the admins verify this patch?

@wbuchwalter
Copy link
Contributor Author

@anhowe @colemickens Do you have any tips to debug a DC/OS deployment?
Deployment is stuck on /waitforleader and ssh'ing into the master is impossible.

@anhowe
Copy link
Contributor

anhowe commented Mar 9, 2017

Being stuck on /waitforleader means that the basic DCOS services are not coming up. This usually points to the fact that the import of custom data, or guid specification was wrong.

To troubleshoot this, I first:

  1. deploy the original DCOS template
  2. once verified that Added a vnet template. #1 works, you must go through the customData of the master and verify the customData of the DCOS template, and the customData of your generated template match in every way
  3. once port 80, 8080 and 443 are HTTP ports #2 is complete, you need to repeat for 3, and 5 master setups

@wbuchwalter
Copy link
Contributor Author

This PR is ready for review.
I have successfully deployed with 1, 3 and 5 masters, and ran GPU and non-GPU containers.
Note that for 1.9, a new NAT rule is needed for SSH, hence the change. (See dcos/dcos@1f41dc6).

var files []string
var baseFile string
if properties.OrchestratorProfile.OrchestratorType == api.DCOS188 ||
properties.OrchestratorProfile.OrchestratorType == api.DCOS190 ||
Copy link
Contributor

Choose a reason for hiding this comment

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

@wbuchwalter , can you also update "if strings.HasPrefix" condition (~line 359) for default "case api.DCOS" to be DCOS190 URL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amanohar Since 1.9 isn't GA yet, I was thinking of letting 1.8.8 as default version so that PR can still be merged, and making another PR once 1.9 hit GA since the bootstrap url will also change from EarlyAccess to Stable and the package ids will be differents.
Let me know what you think.

@JackQuincy
Copy link
Contributor

So that new NAT rule isn't needed. We can still ssh on the same ports as 1.8.8 without it. Now we might want it so that we can ssh to port 22 on the load balancer. I was surprised that your change was different from what is in the dcos repo. You made a loop on master count instead of just a single resource to do this.

@wbuchwalter
Copy link
Contributor Author

@JackQuincy Thanks! I removed the loop that was a mistake indeed.

@macalinao
Copy link

Is this working? DCOS 1.9 has some amazing features, and we'd like to be able to use it.

@wbuchwalter
Copy link
Contributor Author

@macalinao Yes it is working. If you find bug/issues with this PR please let us know.

@anhowe
Copy link
Contributor

anhowe commented Mar 27, 2017

@wbuchwalter - Please refresh to latest 1.9, and refresh build

@wbuchwalter
Copy link
Contributor Author

@anhowe done

Copy link
Contributor

@JackQuincy JackQuincy left a comment

Choose a reason for hiding this comment

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

two little changes other than that lgtm

pkg/api/const.go Outdated
// DCOS is the string constant for DCOS orchestrator type and defaults to DCOS188
DCOS OrchestratorType = "DCOS"
// DCOS187 is the string constant for DCOS 1.8.8 orchestrator type
// DCOS190 is the string constant for DCOS 1.8.8 orchestrator type
Copy link
Contributor

Choose a reason for hiding this comment

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

copy paste error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

const (
// DCOS is the string constant for DCOS orchestrator type and defaults to DCOS188
DCOS = "DCOS"
// DCOS190 is the string constant for DCOS 1.8.8 orchestrator type
Copy link
Contributor

Choose a reason for hiding this comment

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

copy paste error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

Copy link
Contributor

@JackQuincy JackQuincy left a comment

Choose a reason for hiding this comment

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

LGTM

@JackQuincy JackQuincy merged commit 073fde6 into Azure:master Mar 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants