removing _1 suffix

[benjamine]

coming from #342,
this will change container naming to avoid the _1 suffix, the change will be:

container_1 => container
container_2 => container_2
container_3 => container_3
...

I updated all docs I found to reflect this. test

Signed-off-by: Benjamin Eidelman [email protected]

[aanand]

Interesting. I think I'm in favour, but I'd like to be sure we're not breaking anything major. A few things:

1. One thing that definitely will break: existing applications which expect DB_1_PORT-style environment variables. There should be an extra link alias for the non-suffixed container that adds the _1 back, so these continue to work.
2. Looking over the integration tests, it seems we don't actually test what environment variables are provided to the linked container. We should absolutely test that, it's crucial.
3. Could you revert your changes to CHANGES.md? It doesn't make sense to rewrite history :)

[d11wtq]

Off-topic for this PR, but we should really change these docs to the much cleaner:

fig run db /bin/sh -c "psql -h db -U docker"

Also, the /bin/sh -c in this is redundant.

[aanand]

/bin/sh -c is necessary if it's using an environment variable, I think, but not if we switch to hostnames.

[d11wtq]

Yep, make the non-suffixed version an alias to the _1 version and then LGTM. We definitely need to update the docs. Nobody should be relying on these variable names anymore, unless you have a very specific use case for them. The /etc/hosts entry is much easier. And environment variables like $DB_PORT_5432_TCP_PORT to get a number 5432 are just plain bonkers. The port number is in the variable name, making it not variable at all!

[aanand]

ping @bfirsh - any reckons?

[bfirsh]

We should definitely still have db_1, db_2, etc for the sake of having a pattern.

I have conflicted feels about db being an alias to db_1. I like the explicitness of db_1 as "first DB server", but I understand that there is normally just one db container. As long as db_1 is still the primary name, I think I'm alright with this.

In terms of the bonkers environment variables, they are being improved in Docker itself. We shouldn't be using environment variables anyway... you can just reference hostname db_1 and port 5432 and it'll all work anyway. I'll open another ticket about this.

[aanand]

As raised in #229, db_1 is an invalid hostname and causes some applications to break, so there needs to be some kind of alias without an underscore in it, even if it's db-1.

[benjamine]

@aanand I've been busy, but I'll do the fixes you mention
@d11wtq 5432 is not hardcoded in the name, that number in the variable name is saying what the port is internally, which might very well be different when exposed, and that's a docker standard anyway.
@bfirsh db wont be an alias to db_1, there just wouldn't be any db_1, lot of people seem to be getting confused with that, having multiple containers running per image is not a common use case.

[thaJeztah]

@bfirsh I think the problem is that db_1, db_2 is determined by the instance of the project running (scaling), not the nth linked db.

In short, this means that I will have to construct different containers if I scale up my application? I.e if I run a single instance, the db container will be accessible as db_1, for the second instance, I now need to use db_2. This doesn't make sense if I still have a single (on-on-one) link to a database.

It would make sense if I had two linked db containers.

[bfirsh]

Fig supports scaling, so there should be a db_n pattern. If we also want db, it should be an alias.

Let's move the hostname discussion to #360

[benjamine]

undid changes to CHANGES.md.

• how can I create an integration test to test env var names? (should I use one_off? I couldn't find a way to run echo $VARNAME programatically) should we? (isn't docker taking care of testing that container names are translated to env vars (and hosts entries)?
• regarding 'db_1' alias, I don't know how would that be possible, docker creates those variables (and entry hosts) from container names, I don't see a way fig can modify that (add more aliases).
• I don't fully understand how scaling works, specially for linked containers, it seems to me that even for the simple case of web=>db, you need much more than adding env variables (eg. load balancing, db replication). Seems to me (just an opinion) that most people will only ever want 1 container of each service, if that's the case it makes sense to follow Principle of Least Surprise and just use db, and for systems using scaling with multiple containers, given the inherent complexity (like discovering linked containers in the hosts file and do some type of balance load), just take the extra work of doing: container_name[n] = "db" + (n > 1) ? ("_" + n) : ""

[danielkummer]

+1 for a fast merge on this

[prologic]

+1

[padcom]

When can we expect this to be pulled in? It's a life saver for me so a big +1 to this...

[harto]

👍

[bfirsh]

Thanks for the contribution @benjamine, but this has now been fixed in #364!

[bfirsh]

Let me know if there's anything you think is still missing.