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

var-update-trigger: Update only devices that should be running the release that the image env var is part of #1796

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 22 additions & 28 deletions src/features/vars-schema/hooks/vars-update-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,24 +316,26 @@ addEnvHooks('device_service_environment_variable', async (args) => {
];
});

// Normally we don't expect these to return any match, since our clients
// * POST image env vars before the release & images gets marked as successful
// * never PATCH them after that point
// * DELETEs are expected to normally only happen if the release gets DELETED,
// in which case the should_be_running__release will change and trigger an update of its own.
addEnvHooks('image_environment_variable', async (args) => {
// The filter paths here should match the expand path of the state GET.
if (args.req.body.release_image != null) {
return {
image_install: {
should_be_running__release: {
$any: {
$alias: 'ii',
$alias: 'r',
$expr: {
installs__image: {
status: 'success',
release_image: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use contains_image to make this match the state get endpoint, and add a comment to the top of this file that the having the queries match the path the state get endpoint uses to get env vars for a device is the way to go. Essentially this captures if a subset of the state-get query will change so the correct thing is to follow the same path (and also means it'll have the same performance characteristics, which we take real care to optimize for in the state-get)

Copy link
Member Author

@thgreasi thgreasi Oct 18, 2024

Choose a reason for hiding this comment

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

Updated the state GET endoints to use the release_image term form
See: #1800
See: https://github.com/balena-io/balena-api/pull/5381
and added the comment that this query should match the state GET paths as discussed.

$any: {
$alias: 'i',
$alias: 'ri',
$expr: {
i: {
release_image: {
$any: {
$alias: 'ri',
$expr: { ri: { id: args.req.body.release_image } },
},
},
ri: {
id: args.req.body.release_image,
},
},
},
Expand All @@ -351,28 +353,20 @@ addEnvHooks('image_environment_variable', async (args) => {
return [
envVarIds,
(envVarIdsChunk) => ({
image_install: {
should_be_running__release: {
$any: {
$alias: 'ii',
$alias: 'r',
$expr: {
installs__image: {
status: 'success',
release_image: {
$any: {
$alias: 'i',
$alias: 'ri',
$expr: {
i: {
release_image: {
ri: {
image_environment_variable: {
$any: {
$alias: 'ri',
$expr: {
ri: {
image_environment_variable: {
$any: {
$alias: 'e',
$expr: { e: { id: { $in: envVarIdsChunk } } },
},
},
},
},
$alias: 'e',
$expr: { e: { id: { $in: envVarIdsChunk } } },
},
},
},
Expand Down
Loading