Should `Resize()` after `EncryptionLoad()` account for the encryption header space? about go-ceph HOT 7 CLOSED

OK, thanks for letting me know. If you want to you could report it to ceph's tracker (https://tracker.ceph.com). Otherwise I'm going to consider it solved. I will now use this issue tracker to add the test I shared earlier as a regression test. Thanks!

from go-ceph.

Here's a potentially silly question: did you verify that the resize procedure in the docs that you quoted works? If yes, we can probably look into what apis the command line tool might be using that is different from go-ceph. Otherwise, it could possibly be a docs issue.

Thanks!

from go-ceph.

@phlogistonjohn Yes, it does look to me like that resize procedure for the rbd command line tool works as I expect it to from the documentation. Resizing an encrypted image that way will update the effective usable space in the image, not the total size of the image including the encryption header.

from go-ceph.

OK, I took a look at the source for the rbd command line tool's resize action:
https://github.com/ceph/ceph/blob/bed336a3e044e8da9d8137c03ed8f4d9406ad339/src/tools/rbd/action/Resize.cc#L55

I looked over the code and saw the only major differences were uses of image.encryption_load2 and image.resize2 but when I looked closely into those functions they appeared to use the same inner apis and should not exhibit externally different behavior.

After looking at a few other options I decided to try to replicate your test case by adapting the code you shared into the go-ceph tests. Here's a copy-n-paste of the code:

// added to rbd/encryption_test.go
func TestEncryptedResize(t *testing.T) {
	conn := radosConnect(t)
	defer conn.Shutdown()

	poolname := GetUUID()
	err := conn.MakePool(poolname)
	assert.NoError(t, err)
	defer conn.DeletePool(poolname)

	imageName := "resizeme"
	imageSize := uint64(50) * 1024 * 1024
	encOpts := EncryptionOptionsLUKS2{
		Alg:        EncryptionAlgorithmAES256,
		Passphrase: []byte("test-password"),
	}

	t.Run("create", func(t *testing.T) {
		ioctx, err := conn.OpenIOContext(poolname)
		require.NoError(t, err)
		defer ioctx.Destroy()
		err = CreateImage(ioctx, imageName, imageSize, NewRbdImageOptions())
		require.NoError(t, err)

		image, err := OpenImage(ioctx, imageName, NoSnapshot)
		require.NoError(t, err)
		defer image.Close()

		s, err := image.GetSize()
		require.NoError(t, err)
		t.Logf("image size before encryption: %d", s)

		err = image.EncryptionFormat(encOpts)
		require.NoError(t, err)

		s, err = image.GetSize()
		require.NoError(t, err)
		t.Logf("image size after encryption: %d", s)
	})

	t.Run("resize", func(t *testing.T) {
		ioctx, err := conn.OpenIOContext(poolname)
		require.NoError(t, err)
		defer ioctx.Destroy()

		image, err := OpenImage(ioctx, imageName, NoSnapshot)
		require.NoError(t, err)
		defer image.Close()

		err = image.EncryptionLoad(encOpts)
		assert.NoError(t, err)

		s, err := image.GetSize()
		require.NoError(t, err)
		t.Logf("image size before resize: %d", s)
		assert.NotEqual(t, s, imageSize)

		err = image.Resize(imageSize)
		assert.NoError(t, err)

		s, err = image.GetSize()
		require.NoError(t, err)
		t.Logf("image size after encryption: %d", s)
	})
}

Fortunately, or unfortunately, the test passed for me:

=== RUN   TestEncryptedResize
=== RUN   TestEncryptedResize/create
    encryption_test.go:161: image size before encryption: 52428800
    encryption_test.go:168: image size after encryption: 35651584
=== RUN   TestEncryptedResize/resize
    encryption_test.go:185: image size before resize: 35651584
    encryption_test.go:193: image size after encryption: 52428800
--- PASS: TestEncryptedResize (12.67s)
    --- PASS: TestEncryptedResize/create (9.34s)
    --- PASS: TestEncryptedResize/resize (1.99s)
PASS

I did try using one ioctx for both blocks of the test without seeing any difference. But using multiple ioctx is a tad bit closer to the working CLI version because ioctx's aren't shared across cli invocation (neither are connections, but I digress).

Anyway, I'm not sure where we differ. I'll note that I did my run with reef libraries against a reef cluster (based on our CI container image). What versions of the ceph libraries and cluster are you using?

from go-ceph.

I reran the test on quincy and pacific. Both passed.

FWIW, now that we have this test I plan on making a PR out of it to at least serve as a regression test for this use case. But I'll hold off on that for a little while in order not to side-track our conversation too much.

from go-ceph.

Thanks for checking that out, that's a surprising result but it definitely looks like it's an issue on my end. I'm running the go-ceph tests from a machine with quincy client libraries and using a microceph single VM running reef. However when I tested the rbd resize command I did run that from the microceph VM so that would have been reef end-to-end. I tried to run the rbd resize again from the machine with quincy and see if that also produces the behavior I see from go-ceph but that results in the error rbd: unrecognised option '--encryption-passphrase-file' so I guess resizing that way from the command line is probably a recent feature added in reef.

from go-ceph.

I upgraded the machine running go-ceph from quincy to reef and now I see the same result you do from TestEncryptedResize so it seems to me that the quincy client -> reef server was the problem.

from go-ceph.