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

Some issues between v1.0.0 and v1.1.0 #27

Open
ozbillwang opened this issue Jul 4, 2018 · 10 comments · May be fixed by #28
Open

Some issues between v1.0.0 and v1.1.0 #27

ozbillwang opened this issue Jul 4, 2018 · 10 comments · May be fixed by #28

Comments

@ozbillwang
Copy link

ozbillwang commented Jul 4, 2018

I successfully use iam-docker roles for containers running in a Rancher v1.6.x platform finally, but only with an old version v1.0.0, no luck with v1.1.0 and v1.2.0.

Please reference the issue #25 for details.

I don't want to miss the new features and need understand what's the problem between the commits from v1.0.0 to v1.1.0, because I got this issue start from v1.1.0

The only notable change is about new IAM_ROLE environment variable (26680c8)

Seems some bugs in below codes, which was added into v1.1.0

https://github.com/swipely/iam-docker/blame/master/src/docker/container_store.go#L177-L185

-       iamRole, hasKey := container.Config.Labels[iamLabel]
-       if !hasKey {
-               return nil, fmt.Errorf("Unable to find label named '%s' for container: %s", iamLabel, id)
+       iamRole, hasLabel := container.Config.Labels[iamLabel]
+       if !hasLabel {
+               env := dockerClient.Env(container.Config.Env)
+               envRole := env.Get(iamEnvironmentVariable)
+               if envRole != "" {
+                       iamRole = envRole
+               } else {
+                       return nil, fmt.Errorf("Unable to find label named '%s' or environment variable '%s' for container: %s", iamLabel, iamEnvironmentVariable, id)
+               }

@willglynn

Could you take a look?

@willglynn
Copy link
Contributor

willglynn commented Jul 4, 2018

Per @nahiluhmot's comment in #25: are you setting IAM_ROLE on the iam-docker container?

If so, I would expect that to fail in some spectacular fashion, possibly including a panic. I hadn't considered what that would do. It doesn't make any sense to configure the iam-docker container's role since iam-docker is responsible for assuming that role; setting either the environment variable or the label is an infinite regress.

Does iam-docker need to notice this situation and emit a better error message?

@ozbillwang
Copy link
Author

@willglynn

Thanks.

I didn't use IAM_ROLE environment and still get error. I added this variable because I saw the error message related to IAM_ROLE.

You can follow the first error I have, which has no IAM_ROLE when run iam-docker container.

@ozbillwang
Copy link
Author

ozbillwang commented Jul 4, 2018

another finding is, even running successfully with v1.0.0, I can see below error log, but the container keep running with Up status.

2018-07-04T02:22:26Z [docker] Unable to add container event-handler=4 id=42026072677d9a20baf02f06c04fea5bc57309bbf087df5780cbde6397edf154 event=start error="Unable to find label named 'com.swipely.iam-docker.iam-profile' for container: 42026072677d9a20baf02f06c04fea5bc57309bbf087df5780cbde6397edf154"

Compare the error (without IAM_ROLE environment) with v1.1.0 and v1.2.0

2018-07-04T02:21:29Z [docker] Unable to add container event-handler=4 id=c28e1a9c7de3791af0dd942331508e299cd18e2bd65ccd77ea0434cbe2697750 event=start error="Unable to find label named 'com.swipely.iam-docker.iam-profile' or environment variable 'IAM_ROLE' for container: c28e1a9c7de3791af0dd942331508e299cd18e2bd65ccd77ea0434cbe2697750"
panic: runtime error: index out of range

with golang errors and container is stopped.

github.com/swipely/iam-docker/vendor/github.com/fsouza/go-dockerclient.(*Env).Map(0xc420127588, 0xc42007e940)
	/go/src/github.com/swipely/iam-docker/vendor/github.com/fsouza/go-dockerclient/env.go:165 +0x170
github.com/swipely/iam-docker/vendor/github.com/fsouza/go-dockerclient.(*Env).Get(0xc420127588, 0x77f26f, 0x8, 0x22, 0x97fb80)
	/go/src/github.com/swipely/iam-docker/vendor/github.com/fsouza/go-dockerclient/env.go:20 +0x2b
github.com/swipely/iam-docker/src/docker.(*containerStore).findConfigForID(0xc42007e940, 0xc42010b8c0, 0x40, 0x0, 0x7c0ae0, 0xc420302ed0)
	/go/src/github.com/swipely/iam-docker/src/docker/container_store.go:180 +0x536
github.com/swipely/iam-docker/src/docker.(*containerStore).SyncRunningContainers(0xc42007e940, 0x0, 0x0)
	/go/src/github.com/swipely/iam-docker/src/docker/container_store.go:146 +0x290
github.com/swipely/iam-docker/src/app.(*App).syncRunningContainers(0xc420063f80, 0x7c5120, 0xc42007e940, 0x7c2740, 0xc42007e980, 0xc42007ebc0)
	/go/src/github.com/swipely/iam-docker/src/app/app.go:122 +0xba
created by github.com/swipely/iam-docker/src/app.(*App).containerSyncWorker
	/go/src/github.com/swipely/iam-docker/src/app/app.go:52 +0x1af

@willglynn
Copy link
Contributor

Looks like something in go-dockerclient: Env.Get(key) delegates to Env.Map()[key], and Env.Map() can return nil, which means Env.Get() can panic. We can work around it here, but that seems surprising. Digging a bit more…

@willglynn
Copy link
Contributor

No, I'm wrong, dereferencing nil maps is totally fine. env.go line 165 is something to do with parsing the container environment.

Could you run docker ps --format {{.ID}} | xargs docker inspect | jq '.[] | .Config | .Env' and paste the result into a new gist? I think something is formatted wrong somewhere in a container environment, and trying to parse it is the cause of the problem.

@willglynn
Copy link
Contributor

(My hypothesis is that this was the issue in fsouza/go-dockerclient@cd1c311, which is already fixed upstream and needs a dependency bump here in iam-docker.)

@ozbillwang
Copy link
Author

@willglynn

Do you still need the output you asked for? Or you know the root cause already?

@willglynn
Copy link
Contributor

I'm pretty sure it's this, but the raw environment data would confirm.

@ozbillwang
Copy link
Author

@willglynn I run the command, but there are a lot of details with company informations, I can't provide.

willglynn added a commit to willglynn/iam-docker that referenced this issue Jul 12, 2018
willglynn added a commit to willglynn/iam-docker that referenced this issue Jul 12, 2018
@willglynn willglynn linked a pull request Jul 12, 2018 that will close this issue
@willglynn
Copy link
Contributor

Fair enough. Please try my update_dockerclient branch (#28) and see if that fixes the issue.

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 a pull request may close this issue.

2 participants