sqlite: complete Pod methods #17615
Merged
sqlite: complete Pod methods #17615
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[NO NEW TESTS NEEDED] - the sqlite backend is still in development and is not enabled by default. Signed-off-by: Valentin Rothberg <[email protected]>
Otherwise we'll fail immediately as the schema version is returned as 0. Signed-off-by: Valentin Rothberg <[email protected]>
The base table was missing, so we caused a syntax error. Signed-off-by: Valentin Rothberg <[email protected]>
Signed-off-by: Valentin Rothberg <[email protected]>
The value of -1 is used when we do not _yet_ know the exit code of the container. Otherwise, the DB checks would error. There's probably a smarter than allowing -1 but for now, that will do the trick and let the tests progress. Signed-off-by: Valentin Rothberg <[email protected]>
Signed-off-by: Valentin Rothberg <[email protected]>
"TABLE" should refer to the actual table. Signed-off-by: Valentin Rothberg <[email protected]>
The state has been unmarshalled into the config which surfaced in wrong states. Signed-off-by: Valentin Rothberg <[email protected]>
Allow to replace existing exit codes. A container may be started and stopped multiple times etc. Signed-off-by: Valentin Rothberg <[email protected]>
As expected by the system tests. Signed-off-by: Valentin Rothberg <[email protected]>
A number of fixes for pod creation and removal. The important part is that matching partial IDs requires a trailing `%` for SQL to interpret it as a wildcard. More information at https://www.sqlitetutorial.net/sqlite-like/ Signed-off-by: Valentin Rothberg <[email protected]>
To return the error message expected by the system tests. Signed-off-by: Valentin Rothberg <[email protected]>
Requires the trailing `%` to work correctly, see https://www.sqlitetutorial.net/sqlite-like/ Signed-off-by: Valentin Rothberg <[email protected]>
Requires the trailing `%` to work correctly, see
https://www.sqlitetutorial.net/sqlite-like/
Signed-off-by: Valentin Rothberg <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
Note that I'll be buried in meetings until the end of the day. Feel free to take over this PR if needed. |
vrothberg
commented
Feb 23, 2023
| // Set up tables | ||
| if err := sqliteInitTables(state.conn); err != nil { | ||
| return nil, fmt.Errorf("creating tables: %w", err) | ||
| } | ||
|
|
||
| if err := state.migrateSchemaIfNecessary(); err != nil { |
There was a problem hiding this comment.
@mheon there's probably a better way but this change unblocked me.
There was a problem hiding this comment.
Hm. It needs to happen before table init to be safe, so we'll probably have to figure out how to ignore the table-does-not-exist error.
baude
reviewed
Feb 23, 2023
baude
reviewed
Feb 23, 2023
baude
reviewed
Feb 23, 2023
|
code lgtm, couple of nits |
|
Code LGTM |
|
@mheon feel free to merge and take over the comments. I won't find time today. |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@mheon @baude PTAL
This PRs completes the Pod methods and adds a number of fixes. I was chasing the failure in
test/system/220-healthcheck.batsbut ran out of time. Many system and e2e tests are passing, so it's now in a state where we can play on the CLI.Does this PR introduce a user-facing change?