Skip to content

qa/standalone: do not use /etc/fstab as an always-there bytes source#61812

Merged
ronen-fr merged 1 commit intoceph:mainfrom
ronen-fr:wip-rf-fstab
Feb 20, 2025
Merged

qa/standalone: do not use /etc/fstab as an always-there bytes source#61812
ronen-fr merged 1 commit intoceph:mainfrom
ronen-fr:wip-rf-fstab

Conversation

@ronen-fr
Copy link
Contributor

Multiple tests use /etc/fstab whenever a small data file is required as input. After all, as some comments say:
# something that is always there

Alas - it's not always there. Not in containers.

Replacing with a newly-created temporary file filled with random bytes.

Copy link
Contributor

@rzarzynski rzarzynski left a comment

Choose a reason for hiding this comment

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

Good stuff. LGTM with just a single question.
CC: @idryomov, @batrick.

Copy link
Contributor

@idryomov idryomov left a comment

Choose a reason for hiding this comment

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

Not sure why I got CCed, but since I did ;)

local dummyfile=$(file_with_random_data)
# something that is always there
local dummyfile='/etc/fstab'
local dummyfile2='/etc/resolv.conf'
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering why /etc/resolv.conf isn't subjected to the same file_with_random_data treatment. Can /etc/resolv.conf be assumed to always be there, unlike /etc/fstab?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I too was debating whether to change that. But I did not have the pretext to do that: I do not have any reason to believe that resolve.conf may be missing.
But as you have noticed that too - I think I will re-push with this change, too.

@phlogistonjohn
Copy link
Contributor

TIL some tests were using fstab as 'random' data source...

Fascinating ;-)

Copy link
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

otherwise lgtm

local size_bytes=${1:-512}
local file=$(mktemp)
dd if=/dev/urandom of=$file bs=$size_bytes count=1
echo "$file"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
echo "$file"
printf '%s' "$file"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change. Thanks

# a second object (using a different size, for good measure)
local dummyfile2=$(file_with_random_data 1000)
rados -p $poolname put $objname $dummyfile2 || return 1
rm -f $dummyfile2
Copy link
Member

Choose a reason for hiding this comment

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

since this test is not evaluating the handling of an actual input file, you can do this more cleanly:

dd if=/dev/urandom bs=1000 count=1 | rados -p "$poolname" put "$objname" -

Copy link
Member

Choose a reason for hiding this comment

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

(and elsewhere)

(( this is a nit ))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change in all tests where dummyfile2 is used exactly once (as otherwise I'll have to verify that the test is not based on the assumption that the data is the same)

@ronen-fr ronen-fr force-pushed the wip-rf-fstab branch 2 times, most recently from 1288e0c to 1047eff Compare February 16, 2025 14:32
@ronen-fr
Copy link
Contributor Author

jenkins test make check arm64

@ronen-fr ronen-fr force-pushed the wip-rf-fstab branch 2 times, most recently from f9d3290 to e7e200b Compare February 19, 2025 08:08
@ronen-fr
Copy link
Contributor Author

jenkins test api

Multiple tests use /etc/fstab when a small data file is
required as input. After all, as some comments say:
    # something that is always there

Alas - it's not always there. Not in containers.

Replacing with a newly-created temporary file filled with
random bytes.
For completeness - replacing similar references to
/etc/resolv.conf (as a source for random objects) in
the standalone tests, too.

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
@ronen-fr
Copy link
Contributor Author

jenkins test make check arm64

@ronen-fr ronen-fr merged commit e11d6ce into ceph:main Feb 20, 2025
11 checks passed
@ronen-fr ronen-fr deleted the wip-rf-fstab branch February 20, 2025 08:02
zmc pushed a commit to zmc/ceph that referenced this pull request Feb 26, 2025
qa/standalone: do not use /etc/fstab as an always-there bytes source
Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>
Reviewed-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants