Skip to content

Conversation

@rumpl
Copy link
Member

@rumpl rumpl commented Sep 18, 2023

- What I did

  • Skipped TestRemoveImageGarbageCollector, it tests the layer store
  • Fixed TestRemoveContainerAfterLiveRestore, there is no need to pass the storage driver when starting the daemon

- How I did it

- How to verify it

- Description for the changelog

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

This test checks how the layer store works, so we don't need it when we
use containerd as image store

Signed-off-by: Djordje Lukic djordje.lukic@docker.com

This test checks how the layer store works, so we don't need it when we
use containerd as image store

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
@rumpl rumpl added status/2-code-review kind/bugfix PR's that fix bugs containerd-integration Issues and PRs related to containerd integration labels Sep 18, 2023
@rumpl rumpl added this to the 25.0.0 milestone Sep 18, 2023
}

func (s *DockerDaemonSuite) TestRemoveContainerAfterLiveRestore(c *testing.T) {
testRequires(c, DaemonIsLinux, overlayFSSupported, testEnv.IsLocalDaemon)
Copy link
Member

Choose a reason for hiding this comment

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

Is the overlayFSSupported still need if we remove the --storage-driver option?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed ;)

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

There is no need to pass the storage driver to the daemon the test
starts

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
@thaJeztah
Copy link
Member

Failure on Ubuntu is this one (I think I've seen it fail a few times; looks like it's flaky, perhaps we need a tracking ticket if we don't have one yet);

=== Failed
=== FAIL: amd64.integration.daemon TestLiveRestore/volume_references/local_volume_with_mount_options (2.41s)
    daemon_test.go:513: assertion failed: error is not nil: Error response from daemon: remove test-live-restore-volume-references-local: volume has active mounts
        --- FAIL: TestLiveRestore/volume_references/local_volume_with_mount_options (2.41s)

=== FAIL: amd64.integration.daemon TestLiveRestore/volume_references (17.68s)
    --- FAIL: TestLiveRestore/volume_references (17.68s)

=== FAIL: amd64.integration.daemon TestLiveRestore (0.00s)

@rumpl
Copy link
Member Author

rumpl commented Sep 18, 2023

Opened an issue #46508

@thaJeztah thaJeztah merged commit 5d87dc9 into moby:master Sep 18, 2023
@thaJeztah thaJeztah deleted the c8d-fix-tests branch September 18, 2023 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

containerd-integration Issues and PRs related to containerd integration kind/bugfix PR's that fix bugs status/4-merge

Projects

Development

Successfully merging this pull request may close these issues.

4 participants