canonical / microceph

Ceph for a one-rack cluster and appliances
https://snapcraft.io/microceph
GNU Affero General Public License v3.0
193 stars 27 forks source link

Fix storage unit test #302

Closed sabaini closed 4 months ago

sabaini commented 4 months ago

Edit: adding a local unit test run as the CI for unittests is broken:

peter@pirx ~/src/microceph/microceph ±fix/storage-unit-test » make check-unit 
go test ./...
?       github.com/canonical/microceph/microceph/api    [no test files]
?       github.com/canonical/microceph/microceph/api/types  [no test files]
?       github.com/canonical/microceph/microceph/client [no test files]
?       github.com/canonical/microceph/microceph/cmd/microcephd [no test files]
?       github.com/canonical/microceph/microceph/constants  [no test files]
?       github.com/canonical/microceph/microceph/database   [no test files]
?       github.com/canonical/microceph/microceph/interfaces [no test files]
?       github.com/canonical/microceph/microceph/mocks  [no test files]
?       github.com/canonical/microceph/microceph/tests  [no test files]
?       github.com/canonical/microceph/microceph/version    [no test files]
ok      github.com/canonical/microceph/microceph/ceph   0.021s
ok      github.com/canonical/microceph/microceph/cmd/microceph  0.006s
ok      github.com/canonical/microceph/microceph/common 0.006s
UtkarshBhatthere commented 4 months ago

This fixes the unit test: However, this PR is getting strange Error: unable to list system disks: Failed to read "/proc/self/mountinfo": open /proc/self/mountinfo: permission denied errors in CI as well. Anything that you could think of off the top of your head ?

sabaini commented 4 months ago

This fixes the unit test: However, this PR is getting strange Error: unable to list system disks: Failed to read "/proc/self/mountinfo": open /proc/self/mountinfo: permission denied errors in CI as well. Anything that you could think of off the top of your head ?

Smells like an lxc confinement issue which seems to be triggered by the other PR?

UtkarshBhatthere commented 4 months ago

ig you're right, i will pull the unit test fixing commits and apply them onto your PR. This was we can get the urgently needed stuff in fast.

UtkarshBhatthere commented 4 months ago

Hmm, @sabaini this appears to change the test behaviour. Can you take a look at the commits i cherry-picked to see what could be causing the permission denied?

sabaini commented 4 months ago

@UtkarshBhatthere can we please get one PR merged and then cherry pick?

UtkarshBhatthere commented 4 months ago

@sabaini couldn't get back to this yesterday. I have remove the commits from top, waiting for CI to merge this.