Project

General

Profile

Actions

Bug #22433

closed

arvados/build/rails-package-scripts/postinst.sh should exit nonzero if there is a database setup or migration error

Added by Peter Amstutz about 1 year ago. Updated about 1 year ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Deployment
Target version:
Story points:
-
Release relationship:
Auto

Subtasks 2 (0 open2 closed)

Task #22448: Review 22433-rails-postinst-exit-codeResolvedBrett Smith01/28/2025Actions
Task #22499: Review 22433-test-fixesResolvedBrett Smith01/31/2025Actions
Actions #1

Updated by Peter Amstutz about 1 year ago

  • Position changed from -933177 to -933175
Actions #2

Updated by Brett Smith about 1 year ago

  • Assigned To set to Brett Smith
Actions #3

Updated by Peter Amstutz about 1 year ago

  • Subtask #22448 added
Actions #4

Updated by Brett Smith about 1 year ago

  • Status changed from New to In Progress
Actions #5

Updated by Brett Smith about 1 year ago

22433-rails-postinst-exit-code @ f319b69315264b7b698f9ece7de9943a8af357f2

I'm not running automated tests because nothing in developer-run-tests exercises the postinst script. The real test is going to be whether test-provision still works. As a practical matter, I'm not in a good position right now to build test packages, then run test-provision against it, plus that creates a different set of problems like having tordo install packages with code that hasn't been merged to main. I hope this change is straightforward enough that we can just agree to merge it with the understanding I will deal with the fallout if it causes test-provision to fail.

Consider doing the review with git diff -w.

  • All agreed upon points are implemented / addressed.
    • Yes
  • Anything not implemented (discovered or discussed during work) has a follow-up story.
    • N/A
  • Code is tested and passing, both automated and manual, what manual testing was done is described
    • See above
  • Documentation has been updated.
    • I don't think there's anything to really update. We could maaayyybe update the install guide to include apt spew about the RailsAPI postinst failing, but (a) the exact spew will depend on the version of apt and system configuration, and (b) it'll be a decent block of text that doesn't really tell the reader anything beyond "hey expect to see a bunch of text here." On balance I don't think it's worth it.
  • Behaves appropriately at the intended scale (describe intended scale).
    • No change in scale
  • Considered backwards and forwards compatibility issues between client and server.
    • Intentional compatibility change for reasons discussed in the commit message.
  • Follows our coding standards and GUI style guidelines.
    • N/A (no shell style guide)
Actions #6

Updated by Tom Clegg about 1 year ago

This LGTM, thanks.

Actions #7

Updated by Brett Smith about 1 year ago

Branch merged. Leaving this open to confirm that test-provision still passes after the change.

Actions #8

Updated by Peter Amstutz about 1 year ago

  • Target version changed from Development 2025-01-29 to Development 2025-02-12
Actions #9

Updated by Brett Smith about 1 year ago

22433-test-fixes @ 921bb63e5b809a28fe514b62c9112366a70ec381

One brown paper bag fix to the previous branch. A set of fixes to the package test scripts to accommodate the fact that package install can now fail. Some cleanups to our package Docker build.

  • All agreed upon points are implemented / addressed.
    • Yes
  • Anything not implemented (discovered or discussed during work) has a follow-up story.
    • N/A
  • Code is tested and passing, both automated and manual, what manual testing was done is described
    • In this branch, I can successfully build and test packages on debian12 and rocky8. This is sufficient to go down both the test paths.
  • Documentation has been updated.
    • N/A
  • Behaves appropriately at the intended scale (describe intended scale).
    • No change in scale
  • Considered backwards and forwards compatibility issues between client and server.
    • Necessarily follows from previous agreed changes.
  • Follows our coding standards and GUI style guidelines.
    • N/A (no style guide for these languages)
Actions #10

Updated by Brett Smith about 1 year ago

  • Subtask #22499 added
Actions #11

Updated by Tom Clegg about 1 year ago

921bb63e5b LGTM, thanks.

Actions #12

Updated by Brett Smith about 1 year ago

  • Status changed from In Progress to Resolved

Closing this now that we have a successful test-provision: test-provision: #1089

Actions #13

Updated by Peter Amstutz about 1 year ago

  • Release set to 75
Actions

Also available in: Atom PDF