Skip to content

Conversation

@J0WI
Copy link
Contributor

@J0WI J0WI commented Nov 22, 2018

This PR introduces some breaking changes:
The image will contain 3 releases/tags: apache, fpm, fpm-alpine. apache replaces the current nginx+fpm-alpine release, so it should be an alias to latest. The FPM releases should be used with a HTTP server or image, e.g. nginx. Socurrent customized configurations can be migrated to a nginx:alpine and a linked phpmyadmin:fpm-alpine container.

So this may also break the current testing/release process. Please let me know if further adjustments are required (e.g. I don't know if/how the Makefile is currently used).

@J0WI
Copy link
Contributor Author

J0WI commented Nov 24, 2018

The tests inside the testing container are still broken. Is it worth to keep them?

@ibennetch
Copy link
Member

Maybe you can help with some question I've found, I'm trying to do local development and testing on the docker repository, and I think I should be able to create a new image and container directly from my local git repository:

docker build -t pmadocker .
docker run -name mydocker -d -e PMA_ARBITRARY=1 -p 8080:80 pmadocker

However, the environment variables (PMA_ARBITRARY and PMA_HOST when I tried that) don't seem to make any difference when running my deployed container. The image works, but I can't log in to a remote MySQL connection over TCP/IP because I can't set the host IP. If I deploy from the Docker Hub image, those variables do work as expected. Any idea about that?

@ibennetch
Copy link
Member

It appears that the tests are not severely broken and could be fixed, so I prefer to leave them (although ignoring a failing test is probably worse than not having tests in the first place, but this seems like something we could fix eventually).

I'm not aware of any automated or manual use for the Makefile, but I'm not completely sure.

I'm a little unclear on a couple of things. You said there will be the three releases/tags, so does that mean when creating a container a user would run a command like docker run --name myadmin -d --link mysql_db_server:db -p 8080:80 phpmyadmin/phpmyadmin:apline now (specifying the server type as the last part of the line)? Also, would that interfere with the current deployment of having an edge release for testing the development version? Right now the tags are a bit outdated, which I was working on resolving, but it might be that work would conflict with what you've done here.

This is advanced-level Docker for my introductory-level understanding, so it's taking a bit to wrap my head around some of your suggested changes.

@J0WI
Copy link
Contributor Author

J0WI commented Nov 25, 2018

The image works, but I can't log in to a remote MySQL connection over TCP/IP because I can't set the host IP.

I was not able to reproduce this. I'm getting the interface to enter a custom database server.

You said there will be the three releases/tags, so does that mean when creating a container a user would run a command like docker run --name myadmin -d --link mysql_db_server:db -p 8080:80 phpmyadmin/phpmyadmin:apline now (specifying the server type as the last part of the line)? Also, would that interfere with the current deployment of having an edge release for testing the development version?

Yes, users can now specify the server type as the last part of the line. Otherwise the specified default (latest) is used. Just like they can use phpmyadmin/phpmyadmin:edge, phpmyadmin/phpmyadmin:4.2 or phpmyadmin/phpmyadmin (same as phpmyadmin/phpmyadmin:latest) now.

But I highly recommend to use the same tag schema as other images do.
You may have a look at https://hub.docker.com/r/_/wordpress/.

So the tag schema could be something like:
[PhpMyAdmin version]-[PHP version]-[Variant]-[Variant version]
[PhpMyAdmin version]-[PHP version]-[Variant]
[PHP version]-[Variant]
[PhpMyAdmin version]-[Variant]
[Variant]

I'll also provide a generate-stackbrew-library.sh to generate all tags for the docker library automatically.

It appears that the tests are not severely broken and could be fixed, so I prefer to leave them (although ignoring a failing test is probably worse than not having tests in the first place, but this seems like something we could fix eventually).

I can fix the remaining test, but I don't see the benefit from running all tests again inside a container.

@J0WI J0WI closed this Nov 25, 2018
@J0WI J0WI reopened this Nov 25, 2018
@J0WI
Copy link
Contributor Author

J0WI commented Dec 11, 2018

@ibennetch feel free to ping me with questions

@J0WI
Copy link
Contributor Author

J0WI commented Jan 23, 2019

@ibennetch I just rebased my changes on top of phpMyAdmin 4.8.4.
Would be great if you can have a look.

@stale
Copy link

stale bot commented Mar 24, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 24, 2019
@ibennetch
Copy link
Member

That’s bad timing from the bot because we were just discussing amongst the team how to proceed with the tags. In fact, just yesterday I was pushing to move forward to resolve this.

With no one having any opposition or alternative idea, I plan to merge this when I have a chance.

@stale stale bot removed the wontfix label Mar 24, 2019
@ibennetch ibennetch self-assigned this Mar 24, 2019
@J0WI
Copy link
Contributor Author

J0WI commented Mar 27, 2019

I rebased this on top of current v4.8.5. Feel free to squash the commits.

@ibennetch ibennetch merged commit 78e3cb7 into phpmyadmin:master Mar 29, 2019
@ibennetch
Copy link
Member

Thanks for your contribution and hard work on this. I've just squashed and merged the commits.

I wonder if there's a way to update the version information in one place instead of needing to update four files on each new release.

It would be great to update the documentation in https://github.com/phpmyadmin/docker/blob/master/README.md and https://github.com/phpmyadmin/phpmyadmin/blob/master/doc/setup.rst#installing-using-docker with information about working with the new tags, see #214.

Thanks again.

@J0WI
Copy link
Contributor Author

J0WI commented Mar 29, 2019

Thanks for merging this!
I think this is a good base for further enhancements. Some of them are listed in #56 (comment)

@J0WI J0WI deleted the refactor branch March 29, 2019 08:18
@J0WI
Copy link
Contributor Author

J0WI commented Mar 29, 2019

@ibennetch I think you need to adjust the parameters on DockerHub to enable builds with the new directory structure.

@ibennetch
Copy link
Member

@J0WI I'd like to talk more with you about this, if you'd be willing to reach out to me by email (bennetch at gmail) or on Gitter/Google Talk. Sorry but I couldn't find your email address in your profile to reach out directly.

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