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 linuxkit run azure #1933

Merged
merged 2 commits into from Jun 5, 2017

Conversation

Projects
None yet
5 participants
@radu-matei
Contributor

radu-matei commented May 30, 2017

- What I did
Add linuxkit run azure functionality

- How I did it
Integrate with the Azure Go SDK

@GordonTheTurtle

This comment has been minimized.

Show comment
Hide comment
@GordonTheTurtle

GordonTheTurtle May 30, 2017

Collaborator

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "run-azure" git@github.com:radu-matei/linuxkit.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354307328
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

Collaborator

GordonTheTurtle commented May 30, 2017

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "run-azure" git@github.com:radu-matei/linuxkit.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354307328
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

Show outdated Hide outdated src/cmd/linuxkit/push_azure.go
fmt.Printf("USAGE: %s run azure [options] [name]\n\n", invoked)
fmt.Printf("'name' specifies either the name of an already uploaded\n")
fmt.Printf("VHD image or the full path to a image file which will be\n")
fmt.Printf("uploaded before it is run.\n\n")

This comment has been minimized.

@justincormack

justincormack May 30, 2017

Collaborator

thats a bit odd. push should always take a local file and upload it.

@justincormack

justincormack May 30, 2017

Collaborator

thats a bit odd. push should always take a local file and upload it.

Show outdated Hide outdated src/cmd/linuxkit/push_azure.go
resourceGroupName := flags.String("resourceGroupName", "", "Name of resource group to be used for VM")
accountName := flags.String("accountName", "linuxkitstorage", "Name of the storage account")
imagePath := flags.String("imagePath", "", "Local path of the VHD file to be used as OS image")

This comment has been minimized.

@justincormack

justincormack May 30, 2017

Collaborator

this should just be the last argument on the command line, not a flag - see the other push examples.

@justincormack

justincormack May 30, 2017

Collaborator

this should just be the last argument on the command line, not a flag - see the other push examples.

Show outdated Hide outdated src/cmd/linuxkit/run_azure.go
fmt.Printf("'name' specifies either the name of an already uploaded\n")
fmt.Printf("Azure VM VHD or the full path to a image file which will be\n")
fmt.Printf("uploaded before it is run.\n\n")
fmt.Printf("Options:\n\n")

This comment has been minimized.

@justincormack

justincormack May 30, 2017

Collaborator

yes this is the same help text as above, I think it should be different

@justincormack

justincormack May 30, 2017

Collaborator

yes this is the same help text as above, I think it should be different

@GordonTheTurtle GordonTheTurtle removed the dco/no label May 31, 2017

@justincormack justincormack referenced this pull request May 31, 2017

Closed

Azure support #1939

Show outdated Hide outdated docs/azure.md
> This is a preliminary example image with SSHD and Docker services. In the future, there will be an `azure.yml` file in the `examples` directory
Create a new `dev.yml` file [based on the Azure example](../examples/azure.yml), generate a new SSH key and add it in the `yml`, then `moby build dev.yml`.
Create a new `azure.yml` file [based on the Azure example](../examples/azure.yml), generate a new SSH key and add it in the `yml`, then `moby build azure.yml`.

This comment has been minimized.

@justincormack

justincormack May 31, 2017

Collaborator

you can use the source functionality in the files section to get an ssh key from a file, rather than hard coding it now. (unfortunately this is not yet used by the other examples).

Azure has a metadata service for keys though? Should we add support for this to the metadata package?

@justincormack

justincormack May 31, 2017

Collaborator

you can use the source functionality in the files section to get an ssh key from a file, rather than hard coding it now. (unfortunately this is not yet used by the other examples).

Azure has a metadata service for keys though? Should we add support for this to the metadata package?

Show outdated Hide outdated src/cmd/linuxkit/azure.go
@@ -127,16 +137,14 @@ func createStorageAccount(accountName, location string, resourceGroup resources.
func uploadVMImage(resourceGroupName string, accountName string, imagePath string) {
accountKeys, err := accountsClient.ListKeys(resourceGroupName, accountName)
if err != nil {
fmt.Println(err.Error())
log.Fatalf("Unable to retrieve storage account key")
log.Fatalf("Unable to retrieve storage account key: %s", err.Error())

This comment has been minimized.

@justincormack

justincormack May 31, 2017

Collaborator

you can just use "blah : %v", err which will do the same thing more simply (its what we use elsewhere)

@justincormack

justincormack May 31, 2017

Collaborator

you can just use "blah : %v", err which will do the same thing more simply (its what we use elsewhere)

Show outdated Hide outdated src/cmd/linuxkit/azure.go
AdminPassword: to.StringPtr("DummyPassword!123"),
ComputerName: to.StringPtr(defaultComputerName),
AdminUsername: to.StringPtr(unusedAdminUsername),
AdminPassword: to.StringPtr(unusedPassword),

This comment has been minimized.

@justincormack

justincormack May 31, 2017

Collaborator

Still somewhat concerned about these - where are these documented? What exactly are they passwords for?

@justincormack

justincormack May 31, 2017

Collaborator

Still somewhat concerned about these - where are these documented? What exactly are they passwords for?

This comment has been minimized.

@radu-matei

radu-matei May 31, 2017

Contributor

User creation, SSH and passwords are managed by the Azure Linux Agent, which is not present in the images we create at the moment.

The only purpose for those values right now is for deployment validation. I am still thinking of a better way to achieve this.

@radu-matei

radu-matei May 31, 2017

Contributor

User creation, SSH and passwords are managed by the Azure Linux Agent, which is not present in the images we create at the moment.

The only purpose for those values right now is for deployment validation. I am still thinking of a better way to achieve this.

This comment has been minimized.

@justincormack

justincormack May 31, 2017

Collaborator

Ah yes, ok. I think it is better to make the username and password "unused" and "unused" for now, if they can't be empty.

@justincormack

justincormack May 31, 2017

Collaborator

Ah yes, ok. I think it is better to make the username and password "unused" and "unused" for now, if they can't be empty.

This comment has been minimized.

@justincormack

justincormack May 31, 2017

Collaborator

Or at least put a comment.

@justincormack

justincormack May 31, 2017

Collaborator

Or at least put a comment.

This comment has been minimized.

@radu-matei

radu-matei May 31, 2017

Contributor

Yeah, the password requirements are below (3/4 are mandatory):

Has lower characters
Has upper characters
Has a digit
Has a special character (Regex match [\W_])

Will add a comment.

@radu-matei

radu-matei May 31, 2017

Contributor

Yeah, the password requirements are below (3/4 are mandatory):

Has lower characters
Has upper characters
Has a digit
Has a special character (Regex match [\W_])

Will add a comment.

This comment has been minimized.

@justincormack

justincormack May 31, 2017

Collaborator

ugh, yes a comment would be best then

@justincormack

justincormack May 31, 2017

Collaborator

ugh, yes a comment would be best then

Show outdated Hide outdated src/cmd/linuxkit/push_azure.go
fmt.Printf("'name' specifies the path (absolute or relative) of a\n")
fmt.Printf("VHD image be uploaded to an existing Azure Storage Account\n")
fmt.Printf("Options:\n\n")
flags.PrintDefaults()
}
resourceGroupName := flags.String("resourceGroupName", "", "Name of resource group to be used for VM")
accountName := flags.String("accountName", "linuxkitstorage", "Name of the storage account")
accountName := flags.String("accountName", "", "Name of the storage account")

This comment has been minimized.

@justincormack

justincormack May 31, 2017

Collaborator

this doesn't seem consistent with run below

@justincormack

justincormack May 31, 2017

Collaborator

this doesn't seem consistent with run below

@GordonTheTurtle GordonTheTurtle removed the dco/no label May 31, 2017

Show outdated Hide outdated src/cmd/linuxkit/azure.go
storageAccountKeyArg := fmt.Sprintf("STORAGE_ACCOUNT_KEY=%s", *keys[0].Value)
vhdPath := fmt.Sprintf("VHD_PATH=/vhds/%s", image)
output, err := exec.Command("docker", "run", "-v", dockerMount, "-e", vhdPath, "-e", storageAccountNameArg, "-e", storageAccountKeyArg, "radumatei/azure-vhd-upload:alpine").CombinedOutput()

This comment has been minimized.

@radu-matei

radu-matei May 31, 2017

Contributor

@justincormack The question still remains on the docker dependency.

Is this an urgent matter that I should address right now, or can I tackle other issues related to Azure (like opening ports on the VM, for example)?

@radu-matei

radu-matei May 31, 2017

Contributor

@justincormack The question still remains on the docker dependency.

Is this an urgent matter that I should address right now, or can I tackle other issues related to Azure (like opening ports on the VM, for example)?

This comment has been minimized.

@justincormack

justincormack May 31, 2017

Collaborator

Well I think it needs to be resolved before we merge probably...

@justincormack

justincormack May 31, 2017

Collaborator

Well I think it needs to be resolved before we merge probably...

This comment has been minimized.

@radu-matei

radu-matei May 31, 2017

Contributor

Removed the dependency on docker here

@radu-matei

radu-matei May 31, 2017

Contributor

Removed the dependency on docker here

@rn

This comment has been minimized.

Show comment
Hide comment
@rn

rn May 31, 2017

Member

There shouldn't be an additional commit with the fixes to the previous commit

Member

rn commented May 31, 2017

There shouldn't be an additional commit with the fixes to the previous commit

@@ -0,0 +1,101 @@
kernel:

This comment has been minimized.

@rn

rn May 31, 2017

Member

can you base this on the most recent examples? A lot of the lines are no longer necessary. Also, most of the images used here are outdated.

@rn

rn May 31, 2017

Member

can you base this on the most recent examples? A lot of the lines are no longer necessary. Also, most of the images used here are outdated.

This comment has been minimized.

@radu-matei

radu-matei May 31, 2017

Contributor

Updated example to sshd.yml (for now)

@radu-matei

radu-matei May 31, 2017

Contributor

Updated example to sshd.yml (for now)

flags := flag.NewFlagSet("azure", flag.ExitOnError)
invoked := filepath.Base(os.Args[0])
flags.Usage = func() {
fmt.Printf("USAGE: %s run azure [options] imagePath\n\n", invoked)

This comment has been minimized.

@rn

rn May 31, 2017

Member

it's called imagePath here, but name for push

@rn

rn May 31, 2017

Member

it's called imagePath here, but name for push

@radu-matei

This comment has been minimized.

Show comment
Hide comment
@radu-matei

radu-matei May 31, 2017

Contributor

@rneugeba Can't I just squash the commits just before we merge?

There is still some work to be done here, I would rather commit as I go and squash at the end. Is this ok?

Contributor

radu-matei commented May 31, 2017

@rneugeba Can't I just squash the commits just before we merge?

There is still some work to be done here, I would rather commit as I go and squash at the end. Is this ok?

@rn

This comment has been minimized.

Show comment
Hide comment
@rn

rn May 31, 2017

Member

sure

Member

rn commented May 31, 2017

sure

@justincormack

This comment has been minimized.

Show comment
Hide comment
@justincormack

justincormack Jun 1, 2017

Collaborator

Needs to be rebased (use git fetch upstream; git rebase upstream/master not git merge and then it will be easier to squash and so on later...)

Collaborator

justincormack commented Jun 1, 2017

Needs to be rebased (use git fetch upstream; git rebase upstream/master not git merge and then it will be easier to squash and so on later...)

@ddebroy

This comment has been minimized.

Show comment
Hide comment
@ddebroy

ddebroy Jun 1, 2017

@radu-matei Is it possible to just kick off https://hub.docker.com/r/docker4x/agent-azure/ from the VHD? As we discussed in another forum, it will take care of getting the WALinux Agent (so that the VM shows up with correct status in Portal) and set up SSHD as well. agent-azure is maintained by Docker.

ddebroy commented Jun 1, 2017

@radu-matei Is it possible to just kick off https://hub.docker.com/r/docker4x/agent-azure/ from the VHD? As we discussed in another forum, it will take care of getting the WALinux Agent (so that the VM shows up with correct status in Portal) and set up SSHD as well. agent-azure is maintained by Docker.

@radu-matei

This comment has been minimized.

Show comment
Hide comment
@radu-matei

radu-matei Jun 2, 2017

Contributor

@ddebroy Could you point me to an example where this image is used?

Still, this does not change the way we do linuxkit run azure or linukit push azure, but the way we create images for Azure. So it would affect the example, azure,yml.

Thanks!

Contributor

radu-matei commented Jun 2, 2017

@ddebroy Could you point me to an example where this image is used?

Still, this does not change the way we do linuxkit run azure or linukit push azure, but the way we create images for Azure. So it would affect the example, azure,yml.

Thanks!

@radu-matei

This comment has been minimized.

Show comment
Hide comment
@radu-matei

radu-matei Jun 2, 2017

Contributor

And if we integrate that image (and it is indeed the WALinux Agent), then we can no longer set unused usernames and passwords when creating the VM

Contributor

radu-matei commented Jun 2, 2017

And if we integrate that image (and it is indeed the WALinux Agent), then we can no longer set unused usernames and passwords when creating the VM

@ddebroy

This comment has been minimized.

Show comment
Hide comment
@ddebroy

ddebroy Jun 2, 2017

You can take a peek at /etc/init.d/azure in the Moby VHD for Docker4Azure on how we kick it off. Essentially we do something like this:

export DOCKER_FOR_IAAS_VERSION="17.04.0-ce-azure1"
...
docker run -d \
		--privileged \
		--name agent \
		--ipc host \
		--pid host \
		--net host \
		--uts host \
		--label com.docker.editions.system \
		--restart unless-stopped \
		-e DOCKER_FOR_IAAS_VERSION \
		-v /usr/bin/docker:/usr/local/bin/docker:ro \
		-v /mnt:/mnt \
		-v /etc:/etc \
		-v /var/etc/ssh:/etc/ssh \
		-v /var/etc/hostname:/etc/hostname \
		-v /var/home:/home \
		-v /var/run/docker.sock:/var/run/docker.sock \
		-v /var/log:/var/log \
		-v /lib/modules:/lib/modules \
		-v /lib/firmware:/lib/firmware \
		-v /var/lib/waagent:/var/lib/waagent \
		"docker4x/agent-azure:$DOCKER_FOR_IAAS_VERSION"

ddebroy commented Jun 2, 2017

You can take a peek at /etc/init.d/azure in the Moby VHD for Docker4Azure on how we kick it off. Essentially we do something like this:

export DOCKER_FOR_IAAS_VERSION="17.04.0-ce-azure1"
...
docker run -d \
		--privileged \
		--name agent \
		--ipc host \
		--pid host \
		--net host \
		--uts host \
		--label com.docker.editions.system \
		--restart unless-stopped \
		-e DOCKER_FOR_IAAS_VERSION \
		-v /usr/bin/docker:/usr/local/bin/docker:ro \
		-v /mnt:/mnt \
		-v /etc:/etc \
		-v /var/etc/ssh:/etc/ssh \
		-v /var/etc/hostname:/etc/hostname \
		-v /var/home:/home \
		-v /var/run/docker.sock:/var/run/docker.sock \
		-v /var/log:/var/log \
		-v /lib/modules:/lib/modules \
		-v /lib/firmware:/lib/firmware \
		-v /var/lib/waagent:/var/lib/waagent \
		"docker4x/agent-azure:$DOCKER_FOR_IAAS_VERSION"
@justincormack

This comment has been minimized.

Show comment
Hide comment
@justincormack

justincormack Jun 2, 2017

Collaborator

@ddebroy I think it would make more sense to add the azure agent to this repo as a package here.

Collaborator

justincormack commented Jun 2, 2017

@ddebroy I think it would make more sense to add the azure agent to this repo as a package here.

radu-matei added some commits May 17, 2017

Add Azure push and run
azure: React to change requests

azure: Fix push and run message and update example

azure: Remove docker dependency and upload VHD

Modify %s to %v for Go errors

Signed-off-by: radu-matei <matei.radu94@gmail.com>
Add vendoring for Azure
azure: Add further vendor dependencies

Signed-off-by: radu-matei <matei.radu94@gmail.com>
@radu-matei

This comment has been minimized.

Show comment
Hide comment
@radu-matei

radu-matei Jun 2, 2017

Contributor

Rebased and squashed commits.

Contributor

radu-matei commented Jun 2, 2017

Rebased and squashed commits.

@radu-matei

This comment has been minimized.

Show comment
Hide comment
@radu-matei

radu-matei Jun 5, 2017

Contributor

Is there anything else needed to merge a first version of this?

Contributor

radu-matei commented Jun 5, 2017

Is there anything else needed to merge a first version of this?

@@ -11,6 +11,12 @@ github.com/rneugeba/iso9660wrap 4606f848a055435cdef85305960b0e1bb788d506
github.com/satori/go.uuid b061729afc07e77a8aa4fad0a2fd840958f1942a
github.com/surma/gocpio fcb68777e7dc4ea43ffce871b552c0d073c17495
github.com/vmware/govmomi 6f8ebd89d521d9f9af7a6c2219c4deee511020dd
github.com/Azure/azure-sdk-for-go 26132835cbefa2669a306b777f34b929b56aa0a2
github.com/radu-matei/azure-sdk-for-go 3b12823551999669c9a325a32472508e0af7978e

This comment has been minimized.

@justincormack

justincormack Jun 5, 2017

Collaborator

Why are you using both the official and a forked version?

@justincormack

justincormack Jun 5, 2017

Collaborator

Why are you using both the official and a forked version?

This comment has been minimized.

This comment has been minimized.

@radu-matei

radu-matei Jun 5, 2017

Contributor

Basically, the library used for uploading the VHD is based on an older version of the SDK (that is not backwards compatible), so I needed two versions in vendor.conf.

And since vndr does not accept two vendored dependencies with the same base repo, had to fork them and point a specific (older) commit as the dependency.

@radu-matei

radu-matei Jun 5, 2017

Contributor

Basically, the library used for uploading the VHD is based on an older version of the SDK (that is not backwards compatible), so I needed two versions in vendor.conf.

And since vndr does not accept two vendored dependencies with the same base repo, had to fork them and point a specific (older) commit as the dependency.

This comment has been minimized.

@justincormack

justincormack Jun 5, 2017

Collaborator

ok, lets go with this for now and improve later.

@justincormack

justincormack Jun 5, 2017

Collaborator

ok, lets go with this for now and improve later.

@justincormack justincormack merged commit 4b60965 into linuxkit:master Jun 5, 2017

2 checks passed

ci/datakit/linuxkit-ci All tests passed
Details
dco-signed All commits are signed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment