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.

Sign up for GitHub

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

Jump to bottom

Docker image entrypoint should just be "/quilkin" #583

Closed
markmandel opened this issue Aug 30, 2022 · 3 comments · Fixed by #607
Closed

Docker image entrypoint should just be "/quilkin" #583

markmandel opened this issue Aug 30, 2022 · 3 comments · Fixed by #607
Assignees
@markmandel
Labels
area/user-experience Pertaining to developers trying to use Quilkin, e.g. cli interface, configuration, etc kind/feature New feature or request

Comments

@markmandel
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Context:

quilkin/image/Dockerfile

Line 23 in 0ba88ed

ENTRYPOINT ["/quilkin", "--config", "/etc/quilkin/quilkin.yaml", "run"]

Running Quilkin in xds provider mode requires you to replace the entire ENTRYPOINT, rather than altering arguments, since the entrypoint is setup to run only the proxy mode (which was the only mode at time of writing).

This is possible, but it is an extra point of friction, and it would be super nice if you didn't have to.

Describe the solution you'd like

ENTRYPOINT ["/quilkin"]

Then arguments could be provided for the container to specify what mode you wish to run in, which seems like the most amount of sense.

Describe alternatives you've considered

We could leave it as is, but it seems counter intuitive to how containers usually work 🤷🏻‍♂️

Additional context

There are various tests this would break, but they are all fixable.
@markmandel markmandel added kind/feature New feature or request area/user-experience Pertaining to developers trying to use Quilkin, e.g. cli interface, configuration, etc labels Aug 30, 2022
@XAMPPRocky
Copy link
Collaborator

Yep, fully agree, I've already done that in my local branch.

@markmandel
Copy link
Contributor Author

Making a note that we'd likely also need to update several pieces of documentation with this change as well.

@markmandel markmandel self-assigned this Sep 27, 2022
@markmandel
Copy link
Contributor Author

Working on this right now 👍🏻

markmandel added a commit to markmandel/quilkin that referenced this issue Sep 27, 2022
@markmandel
Docker image entrypoint is now /quilkin
c204098 
* Changed the entrypoint to /quilkin
* Fixed cloudbuild.yaml tests.
* Updates to agones integration tests
* Updated container documentation
* Update Xonotic example, including converting the simple sidecar
  example to command line arguments.

Closes googleforgames#583
@markmandel markmandel mentioned this issue Sep 27, 2022
Merged
markmandel added a commit to markmandel/quilkin that referenced this issue Sep 28, 2022
@markmandel
Docker image entrypoint is now /quilkin
55d5a61 
* Changed the entrypoint to /quilkin
* Fixed cloudbuild.yaml tests.
* Updates to agones integration tests
* Updated container documentation
* Update Xonotic example, including converting the simple sidecar
  example to command line arguments.

Closes googleforgames#583
markmandel added a commit to markmandel/quilkin that referenced this issue Sep 28, 2022
@markmandel
Docker image entrypoint is now /quilkin
9229bce 
* Changed the entrypoint to /quilkin
* Fixed cloudbuild.yaml tests.
* Updates to agones integration tests
* Updated container documentation
* Update Xonotic example, including converting the simple sidecar
  example to command line arguments.

Closes googleforgames#583
markmandel added a commit to markmandel/quilkin that referenced this issue Sep 28, 2022
@markmandel
Docker image entrypoint is now /quilkin
cfb13a2 
* Changed the entrypoint to /quilkin
* Fixed cloudbuild.yaml tests.
* Updates to agones integration tests
* Updated container documentation
* Update Xonotic example, including converting the simple sidecar
  example to command line arguments.

Closes googleforgames#583
markmandel added a commit to markmandel/quilkin that referenced this issue Sep 28, 2022
@markmandel
Docker image entrypoint is now /quilkin
36e06e4 
* Changed the entrypoint to /quilkin
* Fixed cloudbuild.yaml tests.
* Updates to agones integration tests
* Updated container documentation
* Update Xonotic example, including converting the simple sidecar
  example to command line arguments.

Closes googleforgames#583
markmandel added a commit that referenced this issue Sep 28, 2022
@markmandel
Docker image entrypoint is now /quilkin (#607)
736e413 
* Changed the entrypoint to /quilkin
* Fixed cloudbuild.yaml tests.
* Updates to agones integration tests
* Updated container documentation
* Update Xonotic example, including converting the simple sidecar
  example to command line arguments.

Closes #583
XAMPPRocky pushed a commit that referenced this issue Oct 10, 2022
@markmandel @XAMPPRocky
Docker image entrypoint is now /quilkin (#607)
cb30a45 
* Changed the entrypoint to /quilkin
* Fixed cloudbuild.yaml tests.
* Updates to agones integration tests
* Updated container documentation
* Update Xonotic example, including converting the simple sidecar
  example to command line arguments.

Closes #583
XAMPPRocky pushed a commit that referenced this issue Oct 10, 2022
@markmandel @XAMPPRocky
Docker image entrypoint is now /quilkin (#607)
f7f2376 
* Changed the entrypoint to /quilkin
* Fixed cloudbuild.yaml tests.
* Updates to agones integration tests
* Updated container documentation
* Update Xonotic example, including converting the simple sidecar
  example to command line arguments.

Closes #583
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@markmandel markmandel

Labels
area/user-experience Pertaining to developers trying to use Quilkin, e.g. cli interface, configuration, etc kind/feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Docker image entrypoint is now /quilkin markmandel/quilkin
2 participants
@markmandel @XAMPPRocky