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

Building reliability into the K8s ACI Connector demo #103

Open
rbitia wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Building reliability into the K8s ACI Connector demo #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ac337d7
reverted file
Aug 23, 2017
d266ac4
Merge branch 'rbitia-addingk8' of https://github.com/rgardler/acs-dem…
Aug 24, 2017
0e6ac07
Merge branch 'rgardler-rbitia-addingk8' into addingk8
Aug 24, 2017
6d88065
commiting kub env
Aug 29, 2017
330e7b6
added fixes for ACI connector demo, the preq. for kub clusters are wo…
Aug 31, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 13 additions & 26 deletions incubator/aci/create-k8-connector/script.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,17 @@ CLI using the instructions below. Find your subscriptionId with the
az CLI:

Use az to create a Service Principal that can perform operations on
your resource group:
your subscription:

```
SUBSCRIPTION_ID=$(az account show | jq -r '.id')
SP_JSON=$(az ad sp create-for-rbac --role="Contributor" --scopes="/subscriptions/$SUBSCRIPTION_ID/resourceGroups/$SIMDEM_RESOURCE_GROUP")
SP_JSON=$(az ad sp create-for-rbac --role="Contributor" --scopes="/subscriptions/$SUBSCRIPTION_ID")
Copy link
Copy Markdown
Contributor

@SorraTheOrc SorraTheOrc Aug 31, 2017

Choose a reason for hiding this comment

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

Is this really necessary? This is a very powerful SP.

Copy link
Copy Markdown
Contributor Author

@rbitia rbitia Sep 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

We are blocked on the dev side so gonna need this to go through for now :/ - it will stay in incubator but this is what sean is going to end up demoing @seanmck is this okay with you? - to have the sp be at the sub level for the demo...

Copy link
Copy Markdown
Contributor

@SorraTheOrc SorraTheOrc Sep 18, 2017

Choose a reason for hiding this comment

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

OK in incubator with a FIXME, not sure we want to be recommending it to folks though.

```
##Register the app
Copy link
Copy Markdown
Contributor

@SorraTheOrc SorraTheOrc Aug 31, 2017

Choose a reason for hiding this comment

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

Can you please explain what is happening here. What is the "app" and why do we need to register it?

Copy link
Copy Markdown
Contributor Author

@rbitia rbitia Sep 18, 2017

Choose a reason for hiding this comment

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

"resource provider - A service that supplies the resources you can deploy and manage through Resource Manager. Each resource provider offers operations for working with the resources that are deployed. Some common resource providers are Microsoft.Compute, which supplies the virtual machine resource, Microsoft.Storage, which supplies the storage account resource, and Microsoft.Web, which supplies resources related to web apps." So ACI is provided through Container Instance resource manager - usually this linkage is automatic but for some people it might not be

Copy link
Copy Markdown
Contributor

@SorraTheOrc SorraTheOrc Sep 19, 2017

Choose a reason for hiding this comment

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

I meant can you explain it in the doc :-)

Also note you have a typo here (missing space between '#' and 'Register'


```
az provider register -n Microsoft.ContainerInstance
az provider list -o table | grep ContainerInstance
```

## Install the ACI Connector
Expand All @@ -61,7 +67,7 @@ spec:
spec:
containers:
- name: aci-connector
image: microsoft/aci-connector-k8s:latest
image: microsoft/aci-connector-k8s:canary
Copy link
Copy Markdown
Contributor

@SorraTheOrc SorraTheOrc Aug 31, 2017

Choose a reason for hiding this comment

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

This is OK in incubator but we need to pin to a release version before we can graduate.

Copy link
Copy Markdown
Contributor Author

@rbitia rbitia Sep 18, 2017

Choose a reason for hiding this comment

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

One day - yes

imagePullPolicy: Always
env:
- name: AZURE_CLIENT_ID
Expand All @@ -88,6 +94,7 @@ kubectl get nodes
```

## Install the NGINX example
We are creating the pod from a simple yaml file.

```
cat examples/nginx-pod.yaml
Expand All @@ -114,30 +121,10 @@ spec:
kubectl create -f examples/nginx-pod.yaml
```

```
kubectl get pods -o wide
```

Since we need to ensure our Public IPs have been assigned before
proceeding, and because we need the IP number later we'll run a loop
to grab the IP once assinged. This is a little cumbersome but great if
you want to script things. If you are doing this manually you can use
`kubectl get service --wait` to display changes as they happen.

```
NGINX_IP=""
while [ -z $NGINX_IP ]; do sleep 10; NGINX_IP=$(kubectl get service vamp -o jsonpath="{.status.loadBalancer.ingress[*].ip}"); done
Copy link
Copy Markdown
Contributor

@SorraTheOrc SorraTheOrc Aug 31, 2017

Choose a reason for hiding this comment

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

Why are we removing this? Your comment on the PR says it's necessary to remove it but not why. There is no replacement for this logic and thus we are left without demonstrating that the container is actually running.

Copy link
Copy Markdown
Contributor Author

@rbitia rbitia Sep 18, 2017

Choose a reason for hiding this comment

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

I tested the line in bash and it didn't work as expected

Copy link
Copy Markdown
Contributor

@SorraTheOrc SorraTheOrc Sep 18, 2017

Choose a reason for hiding this comment

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

So you don't want to wait for an IP? Isn't it better to fix it rather than remove it entirely? The problem is that you are looking up the wrong service name (it's not 'vamp')

```

Now we have our IP:

```
echo $NGINX_IP
```

Take a look...
This command grabs the pods running on the cluster and we can see that
the Nginx pod is running on the connector which means it's running in as Azure Container Instance.

```
xdg-open $NGINX
kubectl get pods -o wide
Copy link
Copy Markdown
Contributor

@SorraTheOrc SorraTheOrc Aug 31, 2017

Choose a reason for hiding this comment

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

Nope this is not enough for a demo. This says it's running but it doesn't prove it is.

Copy link
Copy Markdown
Contributor Author

@rbitia rbitia Sep 18, 2017

Choose a reason for hiding this comment

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

I don't know if it worked or not - I'll try again though

```

2 changes: 1 addition & 1 deletion kubernetes/env.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"ACS_RESOURCE_GROUP": "acs-k8s-test",
"ACS_CLUSTER_NAME": "acs-k8s-test",
"ACS_DNS_PREFIX": "acs-k8s-test",
"ACS_REGION": "eastus"
"ACS_REGION": "westus"
Copy link
Copy Markdown
Contributor

@SorraTheOrc SorraTheOrc Aug 31, 2017

Choose a reason for hiding this comment

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

Why the change to West US? What is wrong with East US?

}