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

Be compatible with IPv6 system #4

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

Conversation

pGarrabos
Copy link

Be compatible with IPv6 system

@bazhenov
Copy link
Owner

bazhenov commented Feb 26, 2020

Hi. Sorry for the late reply.

First of all thank you for your contribution. While reviewing your changes, I've realised I'm not quite sure about the problem you are facing.

Docker already reading open ports from /proc/self/net/tcp6 file. Why are we moving this reading under isIPv6System guard? Is it failing because of the file is missing in your environment?

@pGarrabos
Copy link
Author

HI,

Thank you for your reply.

My main issue is neither on my host file system and my container file system the /proc/self/net/tcp6 exist. I assumed that this is because my host doesn't yet compatible with IPv6. And so my container can't have IPv6 on non IPv6 host. That is why on isIPv6System i just check if the file exists in my host file system.
I have just made a double check and it seems that the container can be IPv6 compatible even if my host is not. So we need to check is the file exits in the container instead of the host. Sorry my mistake.

@pGarrabos
Copy link
Author

I'm facing another issue (not related to this one) but it can have a huge impact on the lib. I use an Oracle image to do my tests. When I first create the container, in background it configures the database and create the schema. The port is available before my schema are created and so the lib give me back control too early. My only way to know if my database is ready is by reading the output log of the container, i must found :
#########################
DATABASE IS READY TO USE!
#########################
(cf: https://github.com/oracle/docker-images/tree/master/OracleDatabase/SingleInstance)

Do you have any idea how to deal with that? It seams that Junit testContainer has a way to do that :

public GenericContainer containerWithLogWait = new GenericContainer("redis:5.0.3")
.withExposedPorts(6379)
.waitingFor(
Wait.forLogMessage(".Ready to accept connections.\n", 1)
);

(cf: https://www.testcontainers.org/features/startup_and_waits/)

@bazhenov
Copy link
Owner

bazhenov commented Mar 4, 2020

Do you have any idea how to deal with that?

I think it could be done the same way. Just monitoring output of a container and waiting for predefined template to match.

@bazhenov
Copy link
Owner

bazhenov commented Mar 4, 2020

I have just made a double check and it seems that the container can be IPv6 compatible even if my host is not. So we need to check is the file exits in the container instead of the host. Sorry my mistake.

I think this could be easily done using shell. So instead of executing cat /proc/self/net/tcp6 you could execute sh -c "if [ -f /proc/self/net/tcp6 ]; then cat /proc/self/net/tcp6; fi". It only print file if it exists.

@pGarrabos
Copy link
Author

I think it could be done the same way. Just monitoring output of a container and waiting for predefined template to match.

I'm sorry for the late reply but I was quite busy.
I was thinking of extending the @container by adding a new attribute for example startConditions that would be an array of predicate or something like that.
The default value is going to be "wait for ports" but It could be extended or replace by custom conditions (by implementing the predicate).
I think the predicate has to have some parametres like the process or cid or other.

What do you think?

@pGarrabos
Copy link
Author

I think this could be easily done using shell. So instead of executing cat /proc/self/net/tcp6 you could execute sh -c "if [ -f /proc/self/net/tcp6 ]; then cat /proc/self/net/tcp6; fi". It only print file if it exists.

Thank you I'm gonna test It today and repush if it works.

Pierre Garrabos added 2 commits March 6, 2020 16:39
pgarrabos/notIpv6System

Conflicts:
	src/main/java/me/bazhenov/docker/Docker.java
@pGarrabos
Copy link
Author

I think it could be done the same way. Just monitoring output of a container and waiting for predefined template to match.

I'm sorry for the late reply but I was quite busy.
I was thinking of extending the @container by adding a new attribute for example startConditions that would be an array of predicate or something like that.
The default value is going to be "wait for ports" but It could be extended or replace by custom conditions (by implementing the predicate).
I think the predicate has to have some parametres like the process or cid or other.

What do you think?

I did not finish and it's not working yet (i need also to do some refactoring) but just to give you an idea of how I see it : https://github.com/pGarrabos/docker-testng-integration/tree/wip/startconditions

I created a new annotation StartCondition that has 3 parameters Condition class (Condition is an interface that represent a condition that has to be fulfil to assume that the container is ready for the test), parameters that are the parameters for the conditions and timeout.
I also edit the annotation @container to add the condition.

Are you ok with this design or do we need to review it completely ?

@bazhenov
Copy link
Owner

As far as I understand your design is supposed to be used like:

@Container(
  name = "my-container",
  image = "...",
  startConditions = {@StartCondition(condition = PortsStarted.class, ....)}
)

I think it could be simplified both in terms of usage scenario and implementation details. How about:

@Container(
  name = "my-container",
  image = "mysql",
  publish = @Port(3306),
  environment = {"MYSQL_ROOT_PASSWORD=secret"},
  waitForStdoutPatternMatch = "^.*$"
)

It could be used more easily. Also current solution (with waitForAllExposedPorts on a @Container) is more robust in terms of usage because client cannot wait for not exposed port, for example.

@pGarrabos
Copy link
Author

Yes you understood well.

I agree with you that only adding waitForStdoutPatternMatch looks easier but it's less extendable.
Providing an interface to implements for a start condition will allows anybody to create their own conditions if we didn't provide it.
For example, if you look at Junit TestContainer you can see that it has also an option to wait for a specific URL to be available. If we decide to not provide the option, if someone need it, he will be able to create it on his project.

Notice also that TestContainer also give a way to create our own start condition.
Maybe there is a less complex way to provide this extendable mechanism, but with annotation I fear that it will be “wordy”

@bazhenov
Copy link
Owner

Well, maybe we could take best of two words.

What if we implement annotations in my way and internal SPI in your way. So in most cases when client using standard wait conditions he can use simple form of:

@Container(
  ...
  waitForStdoutPatternMatch = "^.*$"
)

but in case client need custom client condition he or she can implement it and use in a programmatic way.

class MyTestCase {

  @BeforeClass
  public void setUp() {
    ContinerDefinition def = new ContinerDefinition("mysql:5.6");
    def.addWaitCondition(new MyCustomWaitCondition("param1", "param2"));
    docker.start(def);
  }
}

I think this should not be a problem in non standard cases. What do you think?

@pGarrabos
Copy link
Author

I think this is a good idea. We can try to implement it.

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.

2 participants