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.

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

should cockroachdb seed directory get built when needed? #547

Closed
davepacheco opened this issue Dec 22, 2021 · 2 comments
Closed

should cockroachdb seed directory get built when needed? #547

davepacheco opened this issue Dec 22, 2021 · 2 comments

Comments

@davepacheco
Copy link
Collaborator

davepacheco commented Dec 22, 2021

While debugging #540, and digging into some unexpected changes to the crdb-base directory, I learned that if you, say:

  1. run cargo test
  2. wipe the $TMPDIR/crdb-base directory
  3. run cargo test

Lots of tests fail with:

$ cargo test -p omicron-nexus
    Finished test [unoptimized + debuginfo] target(s) in 0.32s
     Running unittests (target/debug/deps/omicron_nexus-469bcf46f8ee518e)

running 52 tests
test authn::external::cookies::test::test_parse_cookies_empty_headers ... ok
test authn::external::cookies::test::test_parse_cookies_two_cookie_headers ... ok
test authn::external::cookies::test::test_parse_cookies_ignore_other_headers ... ok
test authn::external::cookies::test::test_parse_cookies_one_cookie ... ok
test authn::external::cookies::test::test_parse_cookies_two_cookie_headers_same_name ... ok
test authn::external::cookies::test::test_parse_cookies_two_cookies ... ok
test authn::external::session_cookie::test::test_get_token_no_header ... ok
test authn::external::session_cookie::test::test_session_cookie_value ... ok
test authn::external::session_cookie::test::test_get_token_other_cookie_present ... ok
test authn::external::session_cookie::test::test_get_token ... ok
test authn::external::spoof::test::test_spoof_header_missing ... ok
test authn::external::spoof::test::test_spoof_header_valid ... ok
test authn::external::spoof::test::test_spoof_reserved_values ... ok
test authn::test::test_internal_users ... ok
test authn::external::spoof::test::test_spoof_header_bad_uuids ... ok
test config::test::test_config_nonexistent ... ok
test config::test::test_config_bad_toml ... ok
test config::test::test_config_empty ... ok
test authn::external::session_cookie::test::test_garbage_cookie ... ok
test authn::external::session_cookie::test::test_expired_cookie_absolute ... ok
test authn::external::session_cookie::test::test_valid_cookie ... ok
test authn::external::session_cookie::test::test_missing_cookie ... ok
test authn::external::test::test_authn_sequence ... ok
test authn::external::session_cookie::test::test_other_cookie ... ok
test authn::external::session_cookie::test::test_expired_cookie_idle ... ok
test db::fixed_data::test::test_builtin_fleet_id_is_valid ... ok
test config::test::test_bad_authn_schemes ... ok
test db::fixed_data::user_builtin::test::test_builtin_user_ids_are_valid ... ok
test db::collection_insert::test::test_verify_query ... ok
test db::subnet_allocation::test::test_verify_query ... ok
test external_api::console_api::test::test_find_file_404_on_directory ... ok
test external_api::console_api::test::test_find_file_404_on_disallowed_ext ... ok
test config::test::test_valid ... ok
test external_api::console_api::test::test_find_file_404_on_nonexistent ... ok
test external_api::console_api::test::test_find_file_404_on_nonexistent_nested ... ok
test external_api::console_api::test::test_find_file_404_on_symlink ... ok
test authz::context::test::test_organization ... FAILED
test external_api::console_api::test::test_find_file_finds_file ... ok
test external_api::console_api::test::test_find_file_wont_follow_symlink ... ok
test context::test::test_background_context ... FAILED
test db::pagination::test::test_paginated_multicolumn_ascending ... FAILED
test db::datastore::test::test_session_methods ... FAILED
test context::test::test_test_context ... FAILED
test db::collection_insert::test::test_collection_not_present ... FAILED
test db::collection_insert::test::test_collection_present ... FAILED
test authz::context::test::test_database ... FAILED
test db::pagination::test::test_paginated_single_column_descending ... FAILED
test db::datastore::test::test_project_creation ... FAILED
test db::pagination::test::test_paginated_single_column_ascending ... FAILED
test db::saga_recovery::test::test_successful_saga_does_not_replay_during_recovery ... FAILED
test db::pagination::test::test_paginated_multicolumn_descending ... FAILED
test db::saga_recovery::test::test_failure_during_saga_can_be_recovered ... FAILED

failures:

---- authz::context::test::test_organization stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_database.21169.1.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_database.21169.1.log"
thread 'authz::context::test::test_organization' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14

---- context::test::test_background_context stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_background_context.21169.2.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_background_context.21169.2.log"
thread 'context::test::test_background_context' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14

---- db::pagination::test::test_paginated_multicolumn_ascending stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_paginated_multicolumn_ascending.21169.8.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_paginated_multicolumn_ascending.21169.8.log"
thread 'db::pagination::test::test_paginated_multicolumn_ascending' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14

---- db::datastore::test::test_session_methods stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_session_methods.21169.7.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_session_methods.21169.7.log"
thread 'db::datastore::test::test_session_methods' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14

---- context::test::test_test_context stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_background_context.21169.3.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_background_context.21169.3.log"
thread 'context::test::test_test_context' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14

---- db::collection_insert::test::test_collection_not_present stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_collection_not_present.21169.4.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_collection_not_present.21169.4.log"
thread 'db::collection_insert::test::test_collection_not_present' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14

---- db::collection_insert::test::test_collection_present stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_collection_present.21169.5.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_collection_present.21169.5.log"
thread 'db::collection_insert::test::test_collection_present' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14

---- authz::context::test::test_database stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_database.21169.0.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_database.21169.0.log"
thread 'authz::context::test::test_database' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- db::pagination::test::test_paginated_single_column_descending stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_paginated_single_column_descending.21169.12.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_paginated_single_column_descending.21169.12.log"
thread 'db::pagination::test::test_paginated_single_column_descending' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14

---- db::datastore::test::test_project_creation stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_project_creation.21169.6.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_project_creation.21169.6.log"
thread 'db::datastore::test::test_project_creation' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14

---- db::pagination::test::test_paginated_single_column_ascending stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_paginated_single_column_ascending.21169.9.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_paginated_single_column_ascending.21169.9.log"
thread 'db::pagination::test::test_paginated_single_column_ascending' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14

---- db::saga_recovery::test::test_successful_saga_does_not_replay_during_recovery stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_successful_saga_does_not_replay_during_recovery.21169.11.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_successful_saga_does_not_replay_during_recovery.21169.11.log"
thread 'db::saga_recovery::test::test_successful_saga_does_not_replay_during_recovery' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14

---- db::pagination::test::test_paginated_multicolumn_descending stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_paginated_multicolumn_descending.21169.10.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_paginated_multicolumn_descending.21169.10.log"
thread 'db::pagination::test::test_paginated_multicolumn_descending' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14

---- db::saga_recovery::test::test_failure_during_saga_can_be_recovered stdout ----
log file: "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_failure_during_saga_can_be_recovered.21169.13.log"
note: configured to log to "/dangerzone/omicron_tmp/omicron_nexus-469bcf46f8ee518e-test_failure_during_saga_can_be_recovered.21169.13.log"
thread 'db::saga_recovery::test::test_failure_during_saga_can_be_recovered' panicked at 'Cannot copy storage from seed directory: Failed to read_dir /dangerzone/omicron_tmp/crdb-base

Caused by:
    No such file or directory (os error 2)', /home/dap/omicron/test-utils/src/dev/mod.rs:139:14


failures:
    authz::context::test::test_database
    authz::context::test::test_organization
    context::test::test_background_context
    context::test::test_test_context
    db::collection_insert::test::test_collection_not_present
    db::collection_insert::test::test_collection_present
    db::datastore::test::test_project_creation
    db::datastore::test::test_session_methods
    db::pagination::test::test_paginated_multicolumn_ascending
    db::pagination::test::test_paginated_multicolumn_descending
    db::pagination::test::test_paginated_single_column_ascending
    db::pagination::test::test_paginated_single_column_descending
    db::saga_recovery::test::test_failure_during_saga_can_be_recovered
    db::saga_recovery::test::test_successful_saga_does_not_replay_during_recovery

test result: FAILED. 38 passed; 14 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

error: test failed, to rerun pass '-p omicron-nexus --lib'

I think this is because we construct the seed at build time and cargo doesn't actually grok that the tests depend on this unusual build artifact. I think ideally it would get built when it was needed (and not rebuilt if not needed). I'm not sure how important this is. When I think about the complexity of trying to synchronize multiple test suite processes attempting to do this at the same time, it doesn't really feel worth solving.

Unless we can deal with this using a cargo:rerun-if-changed=$TMPDIR/crdb-base in the build.rs file that builds the seed?

@smklein
Copy link
Collaborator

smklein commented Dec 22, 2021

I gave the method of cargo:rerun-if-changed=$TMPDIR/crdb-base a shot, the problem here is that it always changes.

Build script runs -> crdb-base is generated -> run build script again, sees that it now exists, so it'll run again. This means the "null build" of tests would take 20 seconds, which is bad.

@smklein
Copy link
Collaborator

smklein commented Dec 27, 2021

Reading through https://doc.rust-lang.org/cargo/reference/build-script-examples.html , I kinda think this is "working as intended" (though it should be better when #563 is merged!).

The example in the cargo book shows a file being generated by a build script, and used within the library itself. This file (hello.rs) is placed in the build script OUT_DIR, but there is no "change detection" tracking this output explicitly. Instead, it's just implicit by existing in the OUT_DIR, as far as I can tell, and the "change detector" declarations track inputs (as we already do today), not outputs.

In light of this, I'm going to close this issue - hopefully by scoping the crdb-base directory to the target/ directory, it may be treated more like an "internal build artifact", and this issue is less problematic.

@smklein smklein closed this as completed Dec 27, 2021
leftwo pushed a commit that referenced this issue Oct 16, 2023
Propolis changes:
PHD: refactor & add support Propolis server "environments" (#547)
Begin making Accessor interface more robust
Update Crucible and Omicron deps for Hakari fixes
Add cloud-init volume generation to standalone
Use specified toolchain version for all GHA checks
Use params to configure rust-toolchain in GHA
Update and lock GHA dependencies

Crucible changes:
Use regions_dataset path for apply_smf (#1000)
Don't unwrap when we can't create a dataset (#992)
Fix tests and update log messages. (#995)
Better backpressure (#990)
Update Rust crate proptest to 1.3.1 (#977)
Read only downstairs can skip Live Repair (#984)
Update Rust crate expectorate to 1.1.0 (#975)
Add trait for `ExtentInner` (#982)
report backpressure in upstairs_info dtrace probe (#987)
Support multiple downstairs operations in GtoS (#985)
leftwo added a commit that referenced this issue Oct 16, 2023
Propolis changes:
PHD: refactor & add support Propolis server "environments" (#547) Begin
making Accessor interface more robust
Update Crucible and Omicron deps for Hakari fixes
Add cloud-init volume generation to standalone
Use specified toolchain version for all GHA checks Use params to
configure rust-toolchain in GHA
Update and lock GHA dependencies

Crucible changes:
Use regions_dataset path for apply_smf (#1000)
Don't unwrap when we can't create a dataset (#992) Fix tests and update
log messages. (#995)
Better backpressure (#990)
Update Rust crate proptest to 1.3.1 (#977)
Read only downstairs can skip Live Repair (#984)
Update Rust crate expectorate to 1.1.0 (#975)
Add trait for `ExtentInner` (#982)
report backpressure in upstairs_info dtrace probe (#987) Support
multiple downstairs operations in GtoS (#985)

---------

Co-authored-by: Alan Hanson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants