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

Remove trustkey id migration and config.TrustKeyPath #44329

Merged
merged 5 commits into from Dec 1, 2022
Merged

Remove trustkey id migration and config.TrustKeyPath #44329

merged 5 commits into from Dec 1, 2022

Conversation

thaJeztah
Copy link
Member

See individual commits for details

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added this to the v-next milestone Oct 19, 2022
@thaJeztah thaJeztah force-pushed the remove_trustkey_id_migration branch 2 times, most recently from 9fb4baf to 5ecabab Compare October 21, 2022 02:09
@thaJeztah
Copy link
Member Author

thaJeztah commented Oct 21, 2022 •
edited

Probably permission issues in the test (or daemon) let me check

=== FAIL: amd64.integration.daemon TestConfigDaemonID (1.66s)
    daemon_test.go:39: [d0dc0ba7608a6] failed to start daemon with arguments [-u unprivilegeduser --preserve-env --preserve-env=PATH XDG_RUNTIME_DIR=/go/src/github.com/docker/docker/bundles/test-integration/TestConfigDaemonID/d0dc0ba7608a6/xdgrun HOME=/home/unprivilegeduser -- dockerd-rootless.sh --data-root /go/src/github.com/docker/docker/bundles/test-integration/TestConfigDaemonID/d0dc0ba7608a6/root --exec-root /tmp/dxr/d0dc0ba7608a6 --pidfile /go/src/github.com/docker/docker/bundles/test-integration/TestConfigDaemonID/d0dc0ba7608a6/docker.pid --userland-proxy=true --containerd-namespace d0dc0ba7608a6 --containerd-plugins-namespace d0dc0ba7608a6p --host unix:///tmp/docker-integration/d0dc0ba7608a6.sock --debug --storage-driver overlay2 --iptables=false] : [d0dc0ba7608a6] daemon exited during startup: exit status 1
    panic.go:522: [d0dc0ba7608a6] daemon is not started


failed to start daemon: 
error loading ID file /go/src/github.com/docker/docker/bundles/test-integration/TestConfigDaemonID/d0dc0ba7608a6/root/engine-id: 
open /go/src/github.com/docker/docker/bundles/test-integration/TestConfigDaemonID/d0dc0ba7608a6/root/engine-id: 
permission denied

@thaJeztah
Copy link
Member Author

Okay, so Chown won't work;

2022-11-27T14:36:24.2453374Z [Error] fatal: failed to start daemon: chown C:\Users\runneradmin\AppData\Local\Temp\moby-root\engine-id: not supported by windows

And rootless still fails;

=== FAIL: amd64.integration.daemon TestConfigDaemonID (1.65s)
    daemon_test.go:39: [d2d2814bde545] failed to start daemon with arguments [-u unprivilegeduser --preserve-env --preserve-env=PATH XDG_RUNTIME_DIR=/go/src/github.com/docker/docker/bundles/test-integration/TestConfigDaemonID/d2d2814bde545/xdgrun HOME=/home/unprivilegeduser -- dockerd-rootless.sh --data-root /go/src/github.com/docker/docker/bundles/test-integration/TestConfigDaemonID/d2d2814bde545/root --exec-root /tmp/dxr/d2d2814bde545 --pidfile /go/src/github.com/docker/docker/bundles/test-integration/TestConfigDaemonID/d2d2814bde545/docker.pid --userland-proxy=true --containerd-namespace d2d2814bde545 --containerd-plugins-namespace d2d2814bde545p --host unix:///tmp/docker-integration/d2d2814bde545.sock --debug --storage-driver overlay2 --iptables=false] : [d2d2814bde545] daemon exited during startup: exit status 1
    panic.go:522: [d2d2814bde545] daemon is not started

Error (formatted for readability) is:

failed to start daemon: error loading ID file /go/src/github.com/docker/docker/bundles/test-integration/TestConfigDaemonID/d2d2814bde545/root/engine-id:
  open /go/src/github.com/docker/docker/bundles/test-integration/TestConfigDaemonID/d2d2814bde545/root/engine-id: permission denied

[rootlesskit:child ] error: command [
    /usr/local/bin/dockerd-rootless.sh \
        --data-root /go/src/github.com/docker/docker/bundles/test-integration/TestConfigDaemonID/d2d2814bde545/root \
        --exec-root /tmp/dxr/d2d2814bde545 \
        --pidfile /go/src/github.com/docker/docker/bundles/test-integration/TestConfigDaemonID/d2d2814bde545/docker.pid \
        --userland-proxy=true \
        --containerd-namespace d2d2814bde545 \
        --containerd-plugins-namespace d2d2814bde545p \
        --host unix:///tmp/docker-integration/d2d2814bde545.sock \
        --debug \
        --storage-driver overlay2 \
        --iptables=false
   ] exited: exit status 1
[rootlesskit:parent] error: child exited: exit status 1

@thaJeztah thaJeztah force-pushed the remove_trustkey_id_migration branch 2 times, most recently from f5aaea8 to 0962d0b Compare November 28, 2022 18:21
@thaJeztah
daemon: remove migrateTrustKeyID()
1981706 
The migration code is in the 22.06 branch, and if we don't migrate
the only side-effect is the daemon's ID being regenerated (as a
UUID).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
daemon/config: remove TrustKeyPath, and local utilities
5cdd6ab 
Turned out that the loadOrCreateTrustKey() utility was doing exactly the
same as libtrust.LoadOrCreateTrustKey(), so making it a thin wrapped. I kept
the tests to verify the behavior, but we could remove them as we only need this
for our integration tests.

The storage location for the generated key was changed (again as we only need
this for some integration tests), so we can remove the TrustKeyPath from the
config.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
distribution: use ad-hoc trustkey for tests
e854b2a 
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
use ad-hoc libtrust key
8feeaec 
This is only used for tests, and the key is not verified anymore, so
instead of creating a key and storing it, we can just use an ad-hoc
one.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
distribution: remove unused RequireSchema2
85fddc0 
It's never set, so we can remove it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@tianon tianon merged commit cd8a090 into moby:master Dec 1, 2022
@thaJeztah thaJeztah deleted the remove_trustkey_id_migration branch December 1, 2022 21:10
@thaJeztah thaJeztah mentioned this pull request Dec 1, 2022
Merged
@thaJeztah thaJeztah mentioned this pull request Apr 5, 2023
Merged
@thaJeztah thaJeztah mentioned this pull request Sep 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tianon tianon tianon approved these changes

@cpuguy83 cpuguy83 cpuguy83 approved these changes

Assignees
No one assigned
Labels
area/daemon impact/deprecation status/2-code-review
Projects
None yet
Milestone
24.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@thaJeztah @tianon @cpuguy83