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

cmd/create, doc/toolbox-create, sh: Add flag --hostname to create command. #573

Closed
wants to merge 1 commit into from

Conversation

likan999
Copy link
Contributor

@likan999 likan999 commented Oct 4, 2020

When SSH to a host, ~/.Xauthority will be used for xauth; the entries in it contains the hostname. If within the toolbox container it uses another hostname, the xauth will fail. Therefore we need to be able to specify the hostname when creating the container.

@vilgotf
Copy link

vilgotf commented Oct 6, 2020

You shouldn't ssh from the toolbox? This (might) break some workflows that depend on the hostname being set to toolbox for the detection of a toolbox environment

@vilgotf
Copy link

vilgotf commented Oct 6, 2020

#563 seems like a better solution imo

@likan999
Copy link
Contributor Author

likan999 commented Oct 7, 2020

To clarify, I am not ssh from within the toolbox. I ssh from a remote host to my host machine with X forwarding, then run toolbox enter; from within the toolbox, if I run any X program, it won't start because of the xauth failure.

likan999 added a commit to likan999/ppa-toolbox that referenced this pull request Oct 30, 2020
…mand.

When SSH to a host, ~/.Xauthority will be used for xauth; the entries in
it contains the hostname. If within the toolbox container it uses
another hostname, the xauth will fail. Therefore we need to be able to
specify the hostname when creating the container.

containers#573
@likan999 likan999 changed the title Pass hostname to toolbox container. cmd/create, doc/toolbox-create, sh: Add flag --hostname to create command. Oct 30, 2020
likan999 added a commit to likan999/ppa-toolbox that referenced this pull request Oct 30, 2020
…mand.

When SSH to a host, ~/.Xauthority will be used for xauth; the entries in
it contains the hostname. If within the toolbox container it uses
another hostname, the xauth will fail. Therefore we need to be able to
specify the hostname when creating the container.

containers#573
@likan999
Copy link
Contributor Author

Addressed comments. Please take another look. Thanks.

@softwarefactory-project-zuul
Copy link

Build failed.

likan999 added a commit to likan999/ppa-toolbox that referenced this pull request Oct 30, 2020
…mand.

When SSH to a host, ~/.Xauthority will be used for xauth; the entries in
it contains the hostname. If within the toolbox container it uses
another hostname, the xauth will fail. Therefore we need to be able to
specify the hostname when creating the container.

containers#573
likan999 added a commit to likan999/ppa-toolbox that referenced this pull request Oct 30, 2020
…mand.

When SSH to a host, ~/.Xauthority will be used for xauth; the entries in
it contains the hostname. If within the toolbox container it uses
another hostname, the xauth will fail. Therefore we need to be able to
specify the hostname when creating the container.

containers#573
likan999 added a commit to likan999/ppa-toolbox that referenced this pull request Oct 30, 2020
…mand.

When SSH to a host, ~/.Xauthority will be used for xauth; the entries in
it contains the hostname. If within the toolbox container it uses
another hostname, the xauth will fail. Therefore we need to be able to
specify the hostname when creating the container.

containers#573
@softwarefactory-project-zuul
Copy link

Build failed.

likan999 added a commit to likan999/ppa-toolbox that referenced this pull request Nov 4, 2020
…mand.

When SSH to a host, ~/.Xauthority will be used for xauth; the entries in
it contains the hostname. If within the toolbox container it uses
another hostname, the xauth will fail. Therefore we need to be able to
specify the hostname when creating the container.

containers#573
@softwarefactory-project-zuul
Copy link

Build failed.

likan999 added a commit to likan999/ppa-toolbox that referenced this pull request Nov 4, 2020
…mand.

When SSH to a host, ~/.Xauthority will be used for xauth; the entries in
it contains the hostname. If within the toolbox container it uses
another hostname, the xauth will fail. Therefore we need to be able to
specify the hostname when creating the container.

containers#573
@likan999
Copy link
Contributor Author

likan999 commented Nov 4, 2020

Addressed comments and resolved conflicts. It is ready for review, please take a look, thanks.

…mand.

When SSH to a host, ~/.Xauthority will be used for xauth; the entries in
it contains the hostname. If within the toolbox container it uses
another hostname, the xauth will fail. Therefore we need to be able to
specify the hostname when creating the container.

containers#573
@softwarefactory-project-zuul
Copy link

Build succeeded.

Base automatically changed from master to main March 25, 2021 22:25
@absenth
Copy link

absenth commented May 13, 2022

This implementation is better than the one I've been trying to put together. I like that this mimics the --hostname command podman accepts.

It would be awesome to see this approved and merged as having a dozen or so toolboxes that all have the prompt $user@toolbox or requiring me to set the hostname after the fact manually is getting old.

@sgallagher
Copy link

This looks completely reasonable and I'd very much like to see this land. I am in the same situation as absenth above: I have multiple toolboxes for different projects/distros and I'd really like to be able to name them sanely and easily.

Copy link

@sgallagher sgallagher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes look completely sane to me.

@debarshiray
Copy link
Member

workflows that depend on the hostname being set to toolbox
for the detection of a toolbox environment

The recommended way to detect a Toolbx environment is to look for the /run/.toolboxenv file. That's also what /etc/profile.d/toolbox.sh uses.

@debarshiray
Copy link
Member

To clarify, I am not ssh from within the toolbox. I ssh from a remote
host to my host machine with X forwarding, then run toolbox enter;
from within the toolbox, if I run any X program, it won't start because
of the xauth failure.

Given the timing, I am tempted to ask if you have seen this commit and whether it improves your situation?

Does the suggestion in #969 to include the host's hostname as a suffix in the container's hostname (eg., toolbox. or something similar) help?

Copy link
Member

@debarshiray debarshiray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the delay. This is implementing #563

For what it's worth, #1007 is a more complete and updated implementation of this idea. It validates the string passed to the --hostname option and has tests.

@@ -390,7 +396,7 @@ func createContainer(container, image, release string, showCommandToEnter bool)
createArgs = append(createArgs, xdgRuntimeDirEnv...)

createArgs = append(createArgs, []string{
"--hostname", "toolbox",
"--hostname", createFlags.hostname,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't directly use createFlags.hostname in createContainer because src/cmd/run.go also uses this function.

@debarshiray
Copy link
Member

This looks completely reasonable and I'd very much like to see
this land. I am in the same situation as absenth above: I have
multiple toolboxes for different projects/distros and I'd really
like to be able to name them sanely and easily.

There are at least two different immediate problems here.

First, there's a need to come up with a better default hostname for the Toolbx containers. See #969, and all the issues and pull requests linked from it, for the discussion. This is often conflated with a desire to make it easier to distinguish one container from another, because the hostname shows up in some very vsible places. eg., in the shell's prompt, the terminal emulator's window chrome, etc.. However, they aren't exactly the same.

However, they aren't exactly the same thing. /run/.containerenv inside Toolbx containers contain some metadata about the containers to help distinguish them better. This metadata might better than relying on the hostname because it's richer and not constrained by the rules governing valid hostnames. See #98, and all the issues and pull requests linked from it, for the discussion.

It's already possible today to use /run/.containerenv and /run/.toolboxenv to come up with one's own shell prompt without making any changes to the Toolbx code. Having said that, it will definitely be good to improve the default prompt that Toolbx provides, which is what #98 is about.

This will probably satisfy a big chunk of users out there.

Then, there's the question of letting users customize the hostname, which is what #563 is about, but it has some problems that need to be figured out.

@debarshiray
Copy link
Member

I am going to have to close this pull request. However, you are welcome to help implement #969

Either way, thanks for your interest in Toolbx and contributing to the discussion!

@debarshiray
Copy link
Member

To clarify, I am not ssh from within the toolbox. I ssh from a remote
host to my host machine with X forwarding, then run toolbox enter;
from within the toolbox, if I run any X program, it won't start because
of the xauth failure.

Given the timing, I am tempted to ask if you have seen this commit and whether it improves your situation?

Does the suggestion in #969 to include the host's hostname as a suffix in the container's hostname (eg., toolbox. or something similar) help?

I am still interested in solving your problem with SSH and X forwarding. I am currently inside a Wayland session, which makes it difficult for me to reproduce your problem. So, feel free to open a new issue with the above details to further investigate 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.

5 participants