Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for framework v1.1.1-1.10.4 #90

Merged
merged 7 commits into from
Jun 19, 2018
Merged

Conversation

rimusz
Copy link
Contributor

@rimusz rimusz commented Jun 15, 2018

includes kubernetes API access via marathon-lb

Fixes #88

@rimusz rimusz requested review from jimmidyson and pires June 15, 2018 12:12
Copy link
Contributor

@bmcustodio bmcustodio left a comment

Choose a reason for hiding this comment

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

@rimusz thank you very much for working on this! I have some remarks/questions but this is looking great!

README.md Outdated
@@ -85,6 +106,8 @@ To deploy a **highly-available** cluster with three (3) private and one (1) publ
}
```

**NOTE:** The **highly-available** mode for a cluster must be chosen when installing the package. Changing the highly-available mode after installing the package is not supported.
Copy link
Contributor

Choose a reason for hiding this comment

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

@rimusz one can change kubernetes.high_availability from false to true (but not the other way around) after installing the package.


## Pre-Requisites

First, make sure your cluster fulfils the [Kubernetes package default requirements](https://docs.mesosphere.com/service-docs/kubernetes/1.0.3-1.9.7/install/#prerequisites/).
First, make sure your cluster fulfils the [Kubernetes package default requirements](https://docs.mesosphere.com/service-docs/kubernetes/1.1.1-1.10.4/install/#prerequisites/).
Copy link
Contributor

Choose a reason for hiding this comment

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

@rimusz make deploy (that we run later) will provision a cluster according to the prerequisites, so what exactly does one need to make sure here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

one has enough cloud resources

Makefile Outdated
.PHONY: setup-cli
setup-cli: check-dcos
@echo "Sleep for 15 seconds ..."
@sleep 15
Copy link
Contributor

Choose a reason for hiding this comment

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

@rimusz why do we need to sleep?

Copy link
Contributor Author

@rimusz rimusz Jun 15, 2018

Choose a reason for hiding this comment

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

to wait for master be ready, specially it happens on AWS

Copy link
Contributor

@pires pires Jun 19, 2018

Choose a reason for hiding this comment

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

Can we poll until the DC/OS master is up instead?

Makefile Outdated
$(call get_public_agent_ip)
$(DCOS_CMD) kubernetes kubeconfig --apiserver-url https://$(PUBLIC_AGENT_IP):6443 --insecure-skip-tls-verify
@echo "Sleep for 25 seconds ..."
@sleep 25
Copy link
Contributor

Choose a reason for hiding this comment

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

@rimusz why do we need to sleep?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same here, better put some delay, as otherwise kubeapi is not available right after the above command

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 poll until the Kubernetes API is up instead?

docs/aws.md Outdated
@@ -73,6 +73,28 @@ For more advanced scenarios, please check the [terraform-dcos documentation for

### Kubernetes configuration

#### RBAC

**NOTE:** By default, it will provision a Kubernetes cluster without [RBAC](https://docs.mesosphere.com/services/kubernetes/1.1.1-1.10.4/authn-and-authz/#rbac) support.
Copy link
Contributor

Choose a reason for hiding this comment

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

@rimusz perhaps s/it/dcos-kubernetes-quickstart.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest replacing it will provision with this quickstart provisions.

docs/aws.md Outdated
@@ -89,6 +111,8 @@ To deploy a **highly-available** cluster with three (3) private and one (1) publ
}
```

**NOTE:** The **highly-available** mode for a cluster must be chosen when installing the package. Changing the highly-available mode after installing the package is not supported.
Copy link
Contributor

Choose a reason for hiding this comment

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

@rimusz please see the comment above about kubernetes.high_availability.

docs/azure.md Outdated
@@ -68,6 +68,27 @@ For more advanced scenarios, please check the [terraform-dcos documentation for

### Kubernetes configuration

#### RBAC

**NOTE:** By default, it will provision a Kubernetes cluster without [RBAC](https://docs.mesosphere.com/services/kubernetes/1.1.1-1.10.4/authn-and-authz/#rbac) support.
Copy link
Contributor

Choose a reason for hiding this comment

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

@rimusz perhaps s/it/dcos-kubernetes-quickstart/.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we repeating text in all docs?

docs/azure.md Outdated
@@ -83,6 +104,8 @@ To deploy a **highly-available** cluster with three (3) private and one (1) publ
}
```

**NOTE:** The **highly-available** mode for a cluster must be chosen when installing the package. Changing the highly-available mode after installing the package is not supported.
Copy link
Contributor

Choose a reason for hiding this comment

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

@rimusz please see the comment above about kubernetes.high_availability.

@@ -0,0 +1,23 @@
{
"id": "/kubeapi-proxy",
Copy link
Contributor

Choose a reason for hiding this comment

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

@rimusz I got kubeapi-proxy as well as marathon-lb. Could we stick to a single one or have that as as an option of the Makefile target?

Copy link
Contributor Author

@rimusz rimusz Jun 19, 2018

Choose a reason for hiding this comment

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

we need both, as kube-proxy is just a dummy marathon app with traffic forward to k8s API and marathon-lb which exposes it to internet

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense.

Copy link
Contributor

@pires pires left a comment

Choose a reason for hiding this comment

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

Please, check my comments above.

Copy link
Contributor

@pires pires left a comment

Choose a reason for hiding this comment

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

I feel we must improve our docs skills but this is already a step forward in supporting the new stuff. @bmcstdio feel free to merge if the PR gets your approval.

Copy link
Contributor

@bmcustodio bmcustodio left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @rimusz!

@pires pires merged commit ac27d88 into master Jun 19, 2018
@pires pires deleted the upgrade-k8s-v1-10-4 branch June 19, 2018 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants