Gateway upgrade: review round 8 fixes
Secret Scan / scan (push) Successful in 7s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Build App (Preview) / test (pull_request) Successful in 4m36s
Secret Scan / scan (pull_request) Successful in 7s
Build App (Preview) / create-release (pull_request) Successful in 4s
Build App (Preview) / build-macos (pull_request) Successful in 3m14s
Build App (Preview) / build-linux (pull_request) Successful in 9m29s
Build App (Preview) / build-windows (pull_request) Successful in 6m51s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Secret Scan / scan (push) Successful in 7s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Build App (Preview) / test (pull_request) Successful in 4m36s
Secret Scan / scan (pull_request) Successful in 7s
Build App (Preview) / create-release (pull_request) Successful in 4s
Build App (Preview) / build-macos (pull_request) Successful in 3m14s
Build App (Preview) / build-linux (pull_request) Successful in 9m29s
Build App (Preview) / build-windows (pull_request) Successful in 6m51s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
- A failed lookup of the preferred local build is "unknown", not "use the registry image", so a transient error can't swap a working local-build gateway onto the upstream image. - One image predicate (`image_switch_pending`) behind both `needs_recreate` and status `image_outdated`, so they agree, including for a container whose image id is unknown. - A failed tag after a successful digest pull is logged, not reported as a failed pull. - Keep Dockerfile parser directives (`# syntax=` etc.) in the build-context hash; hash config.yaml whole. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
7077d36f54
commit
53151dfd42
1 file changed
+63
-29
@@ -79,10 +79,12 @@ fn pinned_litellm_version() -> &'static str {
|
||||
fn local_image_ref() -> String {
|
||||
static REF: OnceLock<String> = OnceLock::new();
|
||||
REF.get_or_init(|| {
|
||||
// config.yaml is hashed whole: a `#` line can sit inside a block
|
||||
// scalar there, and it is rarely edited.
|
||||
let context = sha256_hex(&format!(
|
||||
"{}\0{}",
|
||||
significant_lines(GATEWAY_DOCKERFILE),
|
||||
significant_lines(GATEWAY_DEFAULT_CONFIG)
|
||||
GATEWAY_DEFAULT_CONFIG
|
||||
));
|
||||
format!(
|
||||
"{}:{}-{}",
|
||||
@@ -94,15 +96,25 @@ fn local_image_ref() -> String {
|
||||
.clone()
|
||||
}
|
||||
|
||||
/// The build context minus full-line comments and blank lines, so rewording a
|
||||
/// The Dockerfile minus full-line comments and blank lines, so rewording a
|
||||
/// comment in a later release doesn't make every local build look outdated.
|
||||
/// (Both files use `#` for comments.)
|
||||
/// Parser directives (`# syntax=`, `# escape=`, `# check=`) look like comments
|
||||
/// but change the build, so they are kept.
|
||||
fn significant_lines(text: &str) -> String {
|
||||
text.lines()
|
||||
.map(str::trim_end)
|
||||
.filter(|l| {
|
||||
let t = l.trim_start();
|
||||
!t.is_empty() && !t.starts_with('#')
|
||||
if t.is_empty() {
|
||||
return false;
|
||||
}
|
||||
match t.strip_prefix('#') {
|
||||
None => true,
|
||||
Some(rest) => rest
|
||||
.split_once('=')
|
||||
.map(|(key, _)| key.trim())
|
||||
.is_some_and(|key| matches!(key, "syntax" | "escape" | "check")),
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
@@ -175,31 +187,36 @@ async fn pinned_images() -> Result<PinnedImages, String> {
|
||||
registry: None,
|
||||
})
|
||||
}
|
||||
(Err(e), Ok(Some(registry))) => {
|
||||
log::warn!("{}", e);
|
||||
Ok(PinnedImages {
|
||||
local: None,
|
||||
registry: Some(registry),
|
||||
})
|
||||
}
|
||||
// But a failed lookup of the preferred local build leaves the answer
|
||||
// unknown even when the registry image is there: choosing the
|
||||
// registry image would swap a working local-build gateway over.
|
||||
(Err(e), _) | (_, Err(e)) => Err(e),
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether an existing gateway container runs something other than the image
|
||||
/// `ensure` would start it on. With a pinned image present (`Some(Some(_))`)
|
||||
/// that is exactly when the next start or settings save recreates it. With no
|
||||
/// pinned image (`Some(None)`) it is outdated but `ensure` keeps it as is until
|
||||
/// one is pulled or built. `None` means the image check failed: unknown is
|
||||
/// reported as not outdated rather than guessed.
|
||||
/// Whether the container should be moved onto `pinned` — the image half of
|
||||
/// [`needs_recreate`]. A container whose image id is unknown counts as not on
|
||||
/// it, in both places.
|
||||
fn image_switch_pending(existing_image_id: Option<&str>, pinned: &ChosenImage) -> bool {
|
||||
existing_image_id != Some(pinned.id.as_str())
|
||||
}
|
||||
|
||||
/// Status's `image_outdated`: the container exists and isn't on a pinned
|
||||
/// image. With a pinned image present this is exactly [`image_switch_pending`]
|
||||
/// — what the next start or settings save acts on. With none (`Some(None)`) it
|
||||
/// is outdated, but `ensure` keeps it as is until one is pulled or built.
|
||||
/// `None` means the image check failed: unknown is reported as not outdated
|
||||
/// rather than guessed.
|
||||
fn container_image_outdated(
|
||||
container_exists: bool,
|
||||
container_image_id: Option<&str>,
|
||||
chosen: Option<Option<&ChosenImage>>,
|
||||
) -> bool {
|
||||
match (container_image_id, chosen) {
|
||||
(Some(_), Some(None)) => true,
|
||||
(Some(running), Some(Some(chosen))) => running != chosen.id,
|
||||
_ => false,
|
||||
match chosen {
|
||||
_ if !container_exists => false,
|
||||
Some(Some(pinned)) => image_switch_pending(container_image_id, pinned),
|
||||
Some(None) => true,
|
||||
None => false,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -370,7 +387,7 @@ fn needs_recreate(
|
||||
pinned: Option<&ChosenImage>,
|
||||
) -> bool {
|
||||
existing_fingerprint != fingerprint
|
||||
|| pinned.is_some_and(|p| existing_image_id != Some(p.id.as_str()))
|
||||
|| pinned.is_some_and(|p| image_switch_pending(existing_image_id, p))
|
||||
}
|
||||
|
||||
fn sha256_hex(input: &str) -> String {
|
||||
@@ -395,6 +412,7 @@ pub async fn get_gateway_status(settings: &GatewaySettings) -> Result<GatewaySta
|
||||
None => (false, false),
|
||||
};
|
||||
let image_outdated = container_image_outdated(
|
||||
container_exists,
|
||||
container.as_ref().and_then(|c| c.image_id.as_deref()),
|
||||
chosen.as_ref().map(|c| c.as_ref()),
|
||||
);
|
||||
@@ -912,7 +930,8 @@ where
|
||||
// A pull by digest leaves the image untagged, which `docker image prune`
|
||||
// treats as dangling and deletes. Tag it locally; lookups still go by
|
||||
// digest, so a re-pushed upstream tag changes nothing.
|
||||
get_docker()?
|
||||
// Not fatal: the image is present and will be used either way.
|
||||
if let Err(e) = get_docker()?
|
||||
.tag_image(
|
||||
®istry_image_ref(),
|
||||
Some(bollard::image::TagImageOptions {
|
||||
@@ -921,7 +940,13 @@ where
|
||||
}),
|
||||
)
|
||||
.await
|
||||
.map_err(|e| format!("Pulled the gateway image but could not tag it: {}", e))
|
||||
{
|
||||
log::warn!(
|
||||
"Pulled the gateway image but could not tag it (a prune may remove it): {}",
|
||||
e
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn build_gateway_image<F>(on_progress: F) -> Result<(), String>
|
||||
@@ -1075,13 +1100,17 @@ mod tests {
|
||||
#[test]
|
||||
fn outdated_matches_what_ensure_would_recreate() {
|
||||
let pinned = chosen(®istry_image_ref(), "sha256:new");
|
||||
assert!(!container_image_outdated(Some("sha256:new"), Some(Some(&pinned))));
|
||||
assert!(container_image_outdated(Some("sha256:old"), Some(Some(&pinned))));
|
||||
assert!(!container_image_outdated(true, Some("sha256:new"), Some(Some(&pinned))));
|
||||
assert!(container_image_outdated(true, Some("sha256:old"), Some(Some(&pinned))));
|
||||
// No pinned image at all: whatever runs is not it.
|
||||
assert!(container_image_outdated(Some("sha256:old"), Some(None)));
|
||||
assert!(container_image_outdated(true, Some("sha256:old"), Some(None)));
|
||||
// No container, or the check failed: don't claim it.
|
||||
assert!(!container_image_outdated(None, Some(Some(&pinned))));
|
||||
assert!(!container_image_outdated(Some("sha256:old"), None));
|
||||
assert!(!container_image_outdated(false, None, Some(Some(&pinned))));
|
||||
assert!(!container_image_outdated(true, Some("sha256:old"), None));
|
||||
// A container whose image id is unknown: status and ensure agree
|
||||
// that it isn't on the pinned image.
|
||||
assert!(container_image_outdated(true, None, Some(Some(&pinned))));
|
||||
assert!(needs_recreate("fp", "fp", None, Some(&pinned)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1090,6 +1119,11 @@ mod tests {
|
||||
let b = "# reworded\nFROM x\nRUN y\n# trailing\n";
|
||||
assert_eq!(significant_lines(a), significant_lines(b));
|
||||
assert_ne!(significant_lines(a), significant_lines("FROM x\nRUN z\n"));
|
||||
// A parser directive changes the build.
|
||||
assert_ne!(
|
||||
significant_lines(b),
|
||||
significant_lines(&format!("# syntax=docker/dockerfile:1.7\n{b}"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in new issue
Block a user