Added the ability to get a specific cube's docker Id in the interface. #98

tnine · 2015-02-03T03:35:00Z

Added an internal dockerId. This allows multiple strategies to be configured. Fixes #67.

Note that this now separates the Cube's identifier from the Docker instance name used to start and stop the instance. Existing implementations should still work, since the cube encapsulates the docker instance name, and the default is to use a static naming convention. See the tests in ftest for an example of using a unique naming strategy.

This change is

Added an internal dockerId. This allows multiple strategies to be configured.

…ssue-67

aslakknutsen · 2015-02-03T07:42:16Z

Wondering if this should be reflected in https://github.com/arquillian/arquillian-cube/blob/master/api/src/main/java/org/arquillian/cube/CubeID.java as well. The user can Inject it and if he wants to do 'custom' operations on the Docker Container he would need the 'internal' ID right?

lordofthejars · 2015-02-03T08:41:48Z

The thing that it may have an impact is when reusing an already running container. In this case we should clarify in documentation that if the name is set automatically (unique strategy) then you will not be able to use reuse. For this reason maybe doing in Maven level could be better. But I have not thought deep about it.

aslakknutsen · 2015-02-03T08:48:24Z

@lordofthejars If you can do it on the 'Arquillian level' you can do it on the Maven level. Either by simple Maven resource filtering, by built-in arquillian.xml sys/env property replacement or using the Arquillian built in System properties override: -Darq.extension.docker.property=value

tnine · 2015-02-03T16:27:42Z

Here is my first pass. I was able to encapsulate the dockerId within the cube, so no other component cares about the internal docker id. Let me know what you guys think.

lordofthejars · 2015-02-03T18:09:50Z

We talked with Aslak and we think we should find a way to remap link
section to new container name. Now link is done by using container name so
we will need to remap them as well
El dt., 3 de febr., 2015 a les 17.27 Todd Nine notifications@github.com
va escriure:

Here is my first pass. I was able to encapsulate the dockerId within the
cube, so no other component cares about the internal docker id. Let me know
what you guys think.

—
Reply to this email directly or view it on GitHub
#98 (comment)
.

tnine · 2015-02-03T21:28:46Z

@lordofthejars I agree with you that unique strategy and re-use would be an invalid combination. Maybe in the CubeConfiguration there should be a validation phase and throw an IllegalArgumentException?

@aslakknutsen I may be wrong, but my understanding of the implementation was to push down the docker instance Id into the DockerCube and lower levels. If we adhere to this abstraction, when the user receives events, they can still go to the DockerCubeRegistry and get the instance of their DockerCube by the ID they've specified in the xml file. The events and lookup system still use the user defined string from the yaml for identification, however the instance used within docker may be the static name, or may be unique. I'm beginning to think I should remove the getDockerId() api call from the Cube interface. At first, I thought users would need this. But I'm not sure we should expose this to them. Rather, they should interact with the Cube directly.

Also note, this doesn't deal with the port mapping issue. If I start 2 instances of the same cube from 2 different JVMs, the second will fail due to ports already used in the first instance.

Thoughts?

lordofthejars · 2015-02-04T07:49:10Z

@tnine I was not thinking about re-use but in next case:

         wildfly_database:
              extends: wildfly
              links:
                - database:database
            database:
              image: zhilvis/h2-db
              exposedPorts: [81/tcp, 1521/tcp]
              await:
                strategy: polling

Note that the link section uses the name of the container, so if it is dynamically generated, this link section should be changed as well.

aslakknutsen · 2015-02-04T08:47:21Z

@tnine I completely agree. Hide it as much as possible. But currently we have a 'leak' in the abstractions. You can inject the DockerClientExecutor directly in the test case. Currently we have the CubeID object which is kinda pointless, the only thing it tells you is the 'current' active scoped Cube if you're running it with a Arquillian Container. But CubeID would be e.g. 'my-database' in this case, while if you attempt to use that with the DockerClientExecutor there would be no container with that name as it's been remapped.

Either we need to change the CubeID to be the 'mapped name', or include both cubeId and dockerId.

Or we dump the whole DockerClientExecutor injection, it's really just a silly/dangerous way of by passing any safe guards/automatic cleanup we might have built into Cube core. When we have the CubeController i'm not sure access to DockerClientExecutor makes sense anymore

@lordofthejars ^

tnine · 2015-02-04T16:28:56Z

@aslakknutsen Gotcha. Unfortunately that's beyond my current skill to fix. I'm not sure I'm up to that task without seriously borking something. I may have missed it, but is there an ability to get mapped ports by source ports for a given cube instance? I.E I want to expose 9160, but on my host it maps to 10000. I'll need to know that 9160 has been mapped to 10000 in order to configure my client code.

@lordofthejars If we retain the user defined cubeId and hide the docker instance id into our cube, wouldn't the reference you're using still work? The cubeId will always be static, so you can reference it. Only it's internal docker instance id may change.

lordofthejars · 2015-02-04T18:44:37Z

Well for what I understand of linking process. It is done by using the name given to the container. So if we add a dynamic name then we would need to remap the name of the link as well. If you look on my example database is a name of a container, and we cannot instanciate twice container with same name (I am not sure but it seems it won't have sense)

Also you can get the relation between exposed port and port binding by calling the inspect command and getting HostConfig.

tnine · 2015-02-04T20:20:47Z

@lordofthejars Thanks for the info. I'll take a look at the linking process. It looks like we might need the getDockerId after all. I'm envisioning something like this.

Look up the cube by name
Substitute it in the link with the dockerId.

I haven't found this in the code yet, I'll take a look in a bit. Having this encapsulated within the CubeID seems like a good idea with my current understanding.

tnine · 2015-02-05T00:50:12Z

Hey guys. I'm consistently seeing this in the CubeControllerTest after merging in the latest master.


java.lang.RuntimeException: No command response within timeout of 30000 ms.
    at org.jboss.arquillian.protocol.servlet.runner.ServletCommandService.execute(ServletCommandService.java:60)
    at org.arquillian.cube.impl.client.container.remote.ContainerCubeController.create(ContainerCubeController.java:41)
    at org.arquillian.cube.servlet.CubeControllerTest.should_enrich_test_with_cube_controller_in_container(CubeControllerTest.java:40)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
    at org.jboss.arquillian.junit.Arquillian$6$1.invoke(Arquillian.java:325)
    at org.jboss.arquillian.container.test.impl.execution.LocalTestExecuter.execute(LocalTestExecuter.java:60)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:94)
    at org.jboss.arquillian.core.impl.EventContextImpl.invokeObservers(EventContextImpl.java:99)
    at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:81)
    at org.jboss.arquillian.core.impl.ManagerImpl.fire(ManagerImpl.java:145)
    at org.jboss.arquillian.core.impl.ManagerImpl.fire(ManagerImpl.java:116)
    at org.jboss.arquillian.core.impl.EventImpl.fire(EventImpl.java:67)
    at org.jboss.arquillian.container.test.impl.execution.ContainerTestExecuter.execute(ContainerTestExecuter.java:38)
    ...

I noticed the ss ping is now in the source. I fixed this to use the dockerId instead of the cube Id, but the invocation of start never occurs from within the tomcat container.

@lordofthejars Any ideas?

lordofthejars · 2015-02-05T08:26:20Z

Well I think that the problem may be in DockerClient. We found with @aslakknutsen that when we updated the version of DockerClient there is something that makes that depending on test the Dockerclient connection hung. Maybe this is just what happens. For example we know that running ftest of tomcat_default it runs pretty well.

lordofthejars · 2015-02-05T10:02:35Z

I'll take a look at the linking process.

Cool, in fact links has this format CONTAINER:ALIAS so what we need to do is in link section to a remap of container to the container new name and of course the alias can be updated as well to the same generated value.

tnine · 2015-02-05T15:25:40Z

Ok, I'll take a look at this later today, I'm helping with some other issues at the moment. It also seems the pull is broken as well. Now that the ss command is implemented, I'm going to be able to test the full suite with boot2docker. Are there any tests that are failing for you now on the master branch with a local docker? This way I can bring my PR up to the same state as master.

lordofthejars · 2015-02-05T15:27:08Z

Yes there are some tests that are failing but because of timeout, there is something related with docker-client that makes http pool connection be consumed and enters in a deadlock. BTW if you tell me the issues you are involved I will assign them to you and add in progress label.

tnine · 2015-02-05T18:03:49Z

So I've found the issue. It appears the client now requires credentials when executing the pull command. Did something in the client change, or is this a change to docker hub?

Adds error checking on the docker pull command.

Changed error assertion to only check for valid output. Every other response is an error

tnine · 2015-02-06T01:52:40Z

We're blocked by this. It's a connection exhaustion issue in the pool. If we can turn it up to enough connections, it'll at least give us a workaround to continue testing on boot2docker.

docker-java/docker-java#110

lordofthejars · 2015-02-06T07:51:19Z

Yes Great work :)

aslakknutsen · 2015-02-06T08:13:16Z

@tnine Setting configBuilder.withMaxPerRouteConnections(20) here https://github.com/arquillian/arquillian-cube/blob/master/docker/src/main/java/org/arquillian/cube/impl/docker/DockerClientExecutor.java#L108 works for me. Passes our test suite as well as docker-java.

(just to recomment on the correct issue ;)

lordofthejars · 2015-02-06T09:44:24Z

So now almost all work is done the only thing to finish is the link section. And then we will be able to run tests in parallel, isn't it?

tnine · 2015-02-06T17:02:38Z

Correct. I need to increase the connections, then finish the links. @lordofthejars If you want to perform that work feel free. I'm stuck on some production fixes I have to get to today, so I'm not sure I'll be able to get to this before Monday. We didn't have this issue before the ss ping utility. Is it possible this isn't closing the connection properly?

lordofthejars · 2015-02-06T17:17:23Z

Not sure, in theory not, but you can use the previous strategy by setting type: ping.
Well don't worry about time do it when you can :) now I am fixing some issues.
But again please no pressure fix it when you can, this is open source :)

lordofthejars · 2015-02-16T17:04:15Z

@tnine Hi we have been working pretty hard on fixing some blocking issues. Now we have some time to invest in this issue. We have two options, the first one is accept this issue and then open a new issue about renaming links. But if you have already worked a bit on this, we may wait until you finish it without any problem. No pressure, I only ask you to know the state of the issue.

lordofthejars · 2016-07-20T10:22:28Z

I close the PR since it has been reworked in #392

Todd Nine added 2 commits February 2, 2015 20:30

Added the ability to get a specific cube's docker Id in the interface.

745ffef

Added an internal dockerId. This allows multiple strategies to be configured.

Merge branch 'master' of github.com:arquillian/arquillian-cube into i…

dcabb77

…ssue-67

Updated readme

016def3

aslakknutsen mentioned this pull request Feb 4, 2015

Add operation to get log from container #90

Closed

Todd Nine added 2 commits February 5, 2015 11:06

Fixes new polling to use the docker Id

a3ba04b

Adds error checking on the docker pull command.

Changed configuration to be more declarative with less variables.

735e5d6

Changed error assertion to only check for valid output. Every other response is an error

lordofthejars closed this Jul 20, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added the ability to get a specific cube's docker Id in the interface. #98

Added the ability to get a specific cube's docker Id in the interface. #98

tnine commented Feb 3, 2015 •

edited by bartoszmajsak

aslakknutsen commented Feb 3, 2015

lordofthejars commented Feb 3, 2015

aslakknutsen commented Feb 3, 2015

tnine commented Feb 3, 2015

lordofthejars commented Feb 3, 2015

tnine commented Feb 3, 2015

lordofthejars commented Feb 4, 2015

aslakknutsen commented Feb 4, 2015

tnine commented Feb 4, 2015

lordofthejars commented Feb 4, 2015

tnine commented Feb 4, 2015

tnine commented Feb 5, 2015

lordofthejars commented Feb 5, 2015

lordofthejars commented Feb 5, 2015

tnine commented Feb 5, 2015

lordofthejars commented Feb 5, 2015

tnine commented Feb 5, 2015

tnine commented Feb 6, 2015

lordofthejars commented Feb 6, 2015

aslakknutsen commented Feb 6, 2015

lordofthejars commented Feb 6, 2015

tnine commented Feb 6, 2015

lordofthejars commented Feb 6, 2015

lordofthejars commented Feb 16, 2015

lordofthejars commented Jul 20, 2016

Added the ability to get a specific cube's docker Id in the interface. #98

Added the ability to get a specific cube's docker Id in the interface. #98

Conversation

tnine commented Feb 3, 2015 • edited by bartoszmajsak

aslakknutsen commented Feb 3, 2015

lordofthejars commented Feb 3, 2015

aslakknutsen commented Feb 3, 2015

tnine commented Feb 3, 2015

lordofthejars commented Feb 3, 2015

tnine commented Feb 3, 2015

lordofthejars commented Feb 4, 2015

aslakknutsen commented Feb 4, 2015

tnine commented Feb 4, 2015

lordofthejars commented Feb 4, 2015

tnine commented Feb 4, 2015

tnine commented Feb 5, 2015

lordofthejars commented Feb 5, 2015

lordofthejars commented Feb 5, 2015

tnine commented Feb 5, 2015

lordofthejars commented Feb 5, 2015

tnine commented Feb 5, 2015

tnine commented Feb 6, 2015

lordofthejars commented Feb 6, 2015

aslakknutsen commented Feb 6, 2015

lordofthejars commented Feb 6, 2015

tnine commented Feb 6, 2015

lordofthejars commented Feb 6, 2015

lordofthejars commented Feb 16, 2015

lordofthejars commented Jul 20, 2016

tnine commented Feb 3, 2015 •

edited by bartoszmajsak