From f8e805615448de92fbe961398777df75b851afa8 Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Fri, 31 Jul 2026 16:01:33 -0700 Subject: [PATCH 1/9] Centralize ImageBuilder build planning Replace ad hoc cache checks with graph-based planning shared by matrix generation, build execution, and stale-image detection. Add composable policies, explicit actions, and causal explanations. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d9f1ac1-4713-44b4-a6ba-17cb095984f6 --- documentation/base-image-dependency-flow.md | 4 +- eng/docker-tools/CHANGELOG.md | 21 + eng/docker-tools/DEV-GUIDE.md | 54 ++- .../Build/BuildPlannerTests.cs | 450 ++++++++++++++++++ src/ImageBuilder.Tests/BuildCommandTests.cs | 55 ++- .../GenerateBuildMatrixCommandTests.cs | 135 ++++-- .../GetStaleImagesCommandTests.cs | 42 +- src/ImageBuilder/Build/BuildGraph.cs | 195 ++++++++ src/ImageBuilder/Build/BuildPlan.cs | 60 +++ src/ImageBuilder/Build/BuildPlanner.cs | 344 +++++++++++++ src/ImageBuilder/Build/BuildPolicies.cs | 316 ++++++++++++ src/ImageBuilder/Build/ImageDigestCache.cs | 47 ++ src/ImageBuilder/Commands/BuildCommand.cs | 194 +++++--- .../Commands/GenerateBuildMatrixCommand.cs | 169 ++++--- .../Commands/GetStaleImagesCommand.cs | 134 +----- src/ImageBuilder/ImageBuilder.cs | 3 +- src/ImageBuilder/ImageCacheService.cs | 304 ------------ src/ImageBuilder/ImageDigestCache.cs | 45 -- 18 files changed, 1871 insertions(+), 701 deletions(-) create mode 100644 src/ImageBuilder.Tests/Build/BuildPlannerTests.cs create mode 100644 src/ImageBuilder/Build/BuildGraph.cs create mode 100644 src/ImageBuilder/Build/BuildPlan.cs create mode 100644 src/ImageBuilder/Build/BuildPlanner.cs create mode 100644 src/ImageBuilder/Build/BuildPolicies.cs create mode 100644 src/ImageBuilder/Build/ImageDigestCache.cs delete mode 100644 src/ImageBuilder/ImageCacheService.cs delete mode 100644 src/ImageBuilder/ImageDigestCache.cs diff --git a/documentation/base-image-dependency-flow.md b/documentation/base-image-dependency-flow.md index be02778d7..853c89119 100644 --- a/documentation/base-image-dependency-flow.md +++ b/documentation/base-image-dependency-flow.md @@ -18,8 +18,8 @@ Another part of the automation is an Azure DevOps pipeline that will check wheth For each subscription described in the subscriptions file, the pipeline runs the following process: 1. Loads the manifest and Dockerfile from the subscribed repo. The manifest contains metadata about all the images that are to be produced for the repo, how they are to be tagged, and a bunch of other information. This file, when paired with the content of the repo's Dockerfiles, provides an easily consumable description of the base image tags that are being depended upon. 2. Now that we've got the list of base image tags that we care about, the pipeline makes use of the Docker Registry to get the latest digest values for those tags. -3. Remember that [image info](https://github.com/dotnet/versions/blob/main/build-info/docker/image-info.dotnet-dotnet-docker-main.json) file described above? This is where it comes in handy. The pipeline reads that file to determine which digests the published images are dependent upon and compares them to the latest digests that were retrieved in the previous step. If those values are different, it means the published image is making use of an older version of the image. We need to get it updated! The pipeline keeps track of which images, and their dependent images, need to be rebuilt. -4. If any images need to be rebuilt, the pipeline constructs a special build argument that describes the set of paths to the Dockerfiles that need to be built. It then queues a build for the pipeline referenced by the subscription in the subscriptions file, passing it this build argument. When that pipeline builds the images, it will ensure that the built images are making use of the latest base images, just by the nature of the build process pulling the base image before building. You can see here that there's a virtuous cycle; when the pipeline publishes these updated images, it updates the main image info so that it now references the latest base image digests. That is, until the next base image changes and the process starts all over again. +3. Remember that [image info](https://github.com/dotnet/versions/blob/main/build-info/docker/image-info.dotnet-dotnet-docker-main.json) file described above? This is where it comes in handy. ImageBuilder creates a build plan by comparing each recorded final-stage base image digest to the latest digest retrieved in the previous step. A changed digest produces a `BuildImage` action. That action propagates to every dependent image, and other parents required by those dependents are included in the plan. Each action records its cause, so a dependent image can be traced back through the dependency chain to the base image that changed. +4. If the plan contains work, the pipeline constructs a special build argument that describes the actionable Dockerfile paths. It then queues a build for the pipeline referenced by the subscription in the subscriptions file, passing it this build argument. When that pipeline builds the images, it will ensure that the built images are making use of the latest base images, just by the nature of the build process pulling the base image before building. You can see here that there's a virtuous cycle; when the pipeline publishes these updated images, it updates the main image info so that it now references the latest base image digests. That is, until the next base image changes and the process starts all over again. This entire process is hands-off automation running 7 days a week, allowing consumers of .NET Docker images to be assured they're getting the most up-to-date version of the images they care about. diff --git a/eng/docker-tools/CHANGELOG.md b/eng/docker-tools/CHANGELOG.md index 0936f6feb..f05f2e5b9 100644 --- a/eng/docker-tools/CHANGELOG.md +++ b/eng/docker-tools/CHANGELOG.md @@ -4,6 +4,27 @@ All breaking changes and new features in `eng/docker-tools` will be documented i --- +## 2026-07-31: Centralized ImageBuilder build planning + +ImageBuilder now calculates one explainable build plan for matrix trimming, build execution, and +stale-image detection. Rebuilds propagate through internal image dependencies, and each planned +action records a readable causal chain back to the original cache invalidation. Cache checks +implement one `IBuildPolicy` contract and are composed through `CompositeBuildPolicy`, so new +invalidation sources can be added without changing the planner algorithm. + +Matrix generation, build execution, and stale-image detection now use the same cache rules for +missing image-info, base-image changes, Dockerfile commits, and tag-set changes. + +Cached images with changed platform or shared tags are now retained in the build matrix as +`PublishExistingImage` actions so tag additions, removals, and moves can be published without +rebuilding the image. Normal build configuration does not need to change, but these cases can +produce a build job where they were previously trimmed. + +Code that embeds ImageBuilder must replace `IImageCacheService` with `BuildPlanner`. CLI +arguments and pipeline parameters are unchanged. + +--- + ## 2026-06-11: Configurable per-registry referrer-lookup rate limit - Issue: [#2141](https://github.com/dotnet/docker-tools/issues/2141) diff --git a/eng/docker-tools/DEV-GUIDE.md b/eng/docker-tools/DEV-GUIDE.md index d47fb3ebd..e7e9afbf8 100644 --- a/eng/docker-tools/DEV-GUIDE.md +++ b/eng/docker-tools/DEV-GUIDE.md @@ -361,27 +361,49 @@ The `autobuilder` label is how the infrastructure tracks that the failure cycle --- -### Image Caching +### Build Planning and Image Caching -The infrastructure includes caching to avoid rebuilding images that haven't changed. Caching operates at two levels: +ImageBuilder calculates a build plan before deciding what work to run. Each platform receives one action: -**1. Matrix Trimming (job-level caching)** +- **BuildImage**: run the Docker build. +- **PublishExistingImage**: use the valid published image and continue it through downstream processing because tags or other published metadata changed. +- **UsePublishedImage**: the published image is valid, but this invocation must pull, import, or retag it. +- **NoAction**: the published image is valid and this invocation does not need it locally. -When `trimCachedImagesForMatrix` is enabled, the `generateBuildMatrix` command excludes platforms from the build matrix if they would result in cache hits. This means no build job is even created for those platforms—they're completely skipped. +Each action includes readable reasons. When an image is rebuilt because a dependency changed, the reason links to that dependency's reason. This preserves the complete explanation from a dependent image back to the original cache invalidation. -**2. Build-time Caching** +The same planner is used by: -Even if a platform isn't trimmed from the matrix, the `build` command checks each image against the cache before building. If the image is cached, it outputs `CACHE HIT`, pulls the previously-built image from the registry, and skips the actual Docker build. +1. **Matrix trimming**: `generateBuildMatrix` omits `NoAction` platforms when `trimCachedImagesForMatrix` is enabled. +2. **Build execution**: `build` executes `BuildImage` actions and materializes `PublishExistingImage` and `UsePublishedImage` actions. +3. **Stale-image detection**: `getStaleImages` uses the same checks to identify actionable paths before queueing a build. -#### Cache Conditions +Commands select the manifest-filtered graph. Every target in that graph is evaluated with the same cache rules; the planner owns the build decision and dependency propagation. -An image is considered cached when **both** of the following conditions are true: +Planning code lives in `Microsoft.DotNet.ImageBuilder.Build`: -1. **Base image digest is unchanged** — The digest of the base image (FROM image) matches the digest recorded in the image info file from the last successful publish. If the upstream base image has been updated, this condition fails and the image will be rebuilt. +- `BuildGraph` is the only graph abstraction. Its public dictionaries contain parent, child, and shared-build relationships for each target. Parent edges include platform tags and image-level shared tags. Targets share a build when they have the same Dockerfile, target platform, build arguments, and effective FROM overrides. +- `BuildTarget` represents the current desired definition and retains its `PlatformInfo`, image, and repo context for execution. +- `PlatformData` remains the mutable image-info and published-image model. `BuildPlanner` joins it to graph targets only while creating a plan and records the source target when equivalent targets reuse the same published image. +- `IBuildPolicy` is the only check/policy contract. Each check is a small policy class. `CompositeBuildPolicy` applies every child policy and combines their results, with `BuildImage` taking precedence over `PublishExistingImage`, then `UsePublishedImage`, then `NoAction`. +- The ordered `BuildPlanItem` sequence is the execution input. The build command groups those items by repo and image instead of traversing the manifest again to rediscover work. -2. **Dockerfile commit is unchanged** — The git commit URL for the Dockerfile matches the commit URL recorded in the image info file. If you've modified the Dockerfile, this condition fails and the image will be rebuilt. +Adding an invalidation source, such as package-version metadata or intermediate image dependencies, requires one `IBuildPolicy` implementation and adding it to the composite. Dependency and shared-build propagation remain inside `BuildPlanner`; there is no separate resolver. -Caching compares against the published image info stored in the [versions repo](https://github.com/dotnet/versions). This means caching compares against what's been officially published, not what's in your current branch. +#### Planning Rules + +A previously published image is valid when its final-stage base image digest and Dockerfile commit still match the current values. Missing image metadata, a changed base image, or a changed Dockerfile produces a `BuildImage` action. Any platform or image-level shared tag-set change produces a `PublishExistingImage` action so additions, removals, and moves can be published without rebuilding the image. Matrix generation, build execution, and stale-image detection all use this rule sequence. + +When a platform must be built, all descendants in the caller-selected graph are also built. Other parents needed by those descendants are included as `BuildImage` or `UsePublishedImage` actions. Platforms in the same shared-build group can share published image metadata; if one evaluated target in that group is invalidated, every evaluated target in the group is built. + +The planner traverses shared-build groups from graph roots to leaves. It evaluates targets +sequentially within each group, propagates parent `BuildImage` actions to the group, and then +unifies the group's actions. After all build decisions are final, unchanged direct parents +required by built images are changed from `NoAction` to `UsePublishedImage`. + +`getStaleImages`, matrix generation, and build execution evaluate the same cache checks. + +Planning compares against the published image info stored in the [versions repo](https://github.com/dotnet/versions). This means planning compares against what has been officially published, not only what is in the current branch. #### Disabling Caching @@ -492,16 +514,14 @@ If your Dockerfile path doesn't appear in any of the matrix legs, it was trimmed **How to fix:** Set the `noCache` parameter to `true` when queuing the build. -#### Symptom 3: The build output shows `CACHE HIT` +#### Symptom 3: The build output shows `USING PUBLISHED IMAGE` -If your build job runs but you see `CACHE HIT` in the output of the `Build Images` step and the Dockerfile isn't actually built, the [build-time caching](#image-caching) determined that the image doesn't need to be rebuilt. This is an example of what the output in that step looks like: +If your build job runs but you see `USING PUBLISHED IMAGE` in the output of the `Build Images` step and the Dockerfile isn't actually built, build planning determined that the existing published image is valid but needed by this invocation. This is an example of what the output in that step looks like: ``` -Image info's Dockerfile commit: https://github.com/dotnet/dotnet-buildtools-prereqs-docker/blob/aa85f0dcc3b3d6757c80dc8c2a6f38c290b372cc/src/windowsservercore/ltsc2025/helix/amd64/Dockerfile -Latest Dockerfile commit: https://github.com/dotnet/dotnet-buildtools-prereqs-docker/blob/aa85f0dcc3b3d6757c80dc8c2a6f38c290b372cc/src/windowsservercore/ltsc2025/helix/amd64/Dockerfile -Dockerfile commits match: True +Build plan for src/windowsservercore/ltsc2025/helix/amd64/Dockerfile: UsePublishedImage. Base image 'mcr.microsoft.com/windows/servercore:ltsc2025' is unchanged at 'sha256:...'. Dockerfile is unchanged at 'https://github.com/dotnet/dotnet-buildtools-prereqs-docker/blob/.../Dockerfile'. -CACHE HIT +USING PUBLISHED IMAGE -- EXECUTING: docker pull mcr.microsoft.com/dotnet-buildtools/prereqs@sha256:40d36a0aab610f4d513ed7c7300a5d962968a547ffe8a859a0e599691b74b77f ``` diff --git a/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs b/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs new file mode 100644 index 000000000..4110b8d57 --- /dev/null +++ b/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs @@ -0,0 +1,450 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.DotNet.ImageBuilder.Build; +using Microsoft.DotNet.ImageBuilder.Commands; +using Microsoft.DotNet.ImageBuilder.Models.Image; +using Microsoft.DotNet.ImageBuilder.Models.Manifest; +using Microsoft.DotNet.ImageBuilder.Tests.Helpers; +using Microsoft.DotNet.ImageBuilder.ViewModel; +using Moq; +using Newtonsoft.Json; +using Shouldly; +using static Microsoft.DotNet.ImageBuilder.Tests.Helpers.DockerfileHelper; +using static Microsoft.DotNet.ImageBuilder.Tests.Helpers.ManifestHelper; + +namespace Microsoft.DotNet.ImageBuilder.Tests.Build; + +[TestClass] +public class BuildPlannerTests +{ + [TestMethod] + public async Task ChangedBaseImageExplainsFullDependencyChain() + { + using TempFolderContext tempFolder = TestHelper.UseTempFolder(); + ManifestInfo manifest = LoadManifest( + tempFolder, + CreateManifest( + CreateRepo("root", CreateImage(CreatePlatform( + CreateDockerfile("root", tempFolder, "base:tag"), ["tag"]))), + CreateRepo("middle", CreateImage(CreatePlatform( + CreateDockerfile("middle", tempFolder, "root:tag"), ["tag"]))), + CreateRepo("support", CreateImage(CreatePlatform( + CreateDockerfile("support", tempFolder, "support-base:tag"), ["tag"]))), + CreateRepo("leaf", CreateImage(CreatePlatform( + CreateDockerfile("leaf", tempFolder, "support:tag", "middle:tag"), ["tag"]))))); + ImageArtifactDetails imageInfo = CreatePublishedImages( + manifest, + new Dictionary + { + ["root"] = "base@sha256:old", + ["middle"] = "root@sha256:root", + ["support"] = "support-base@sha256:support-base", + ["leaf"] = "middle@sha256:middle", + }); + BuildGraph graph = BuildGraph.Create(manifest); + Mock manifestService = new(); + manifestService + .Setup(service => service.GetManifestDigestShaAsync( + It.Is(image => image.Contains("support-base", StringComparison.Ordinal)), + false)) + .ReturnsAsync("sha256:support-base"); + manifestService + .Setup(service => service.GetManifestDigestShaAsync( + It.Is(image => !image.Contains("support-base", StringComparison.Ordinal)), + false)) + .ReturnsAsync("sha256:new"); + + IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + graph, + imageInfo, + CompositeBuildPolicy.ImageCache( + BuildAction.NoAction, + CreateBaseImageRule(manifest, manifestService.Object))); + + BuildPlanItem root = GetItem(plan, "root"); + BuildPlanItem middle = GetItem(plan, "middle"); + BuildPlanItem support = GetItem(plan, "support"); + BuildPlanItem leaf = GetItem(plan, "leaf"); + + root.Action.ShouldBe(BuildAction.BuildImage); + middle.Action.ShouldBe(BuildAction.BuildImage); + leaf.Action.ShouldBe(BuildAction.BuildImage); + support.Action.ShouldBe(BuildAction.UsePublishedImage); + + BuildReason leafReason = leaf.Reasons.Last(reason => + reason.Message.StartsWith("Dependency", StringComparison.Ordinal)); + BuildReason middleReason = leafReason.Cause.ShouldNotBeNull(); + middleReason.Message.ShouldStartWith("Dependency"); + BuildReason rootReason = middleReason.Cause.ShouldNotBeNull(); + rootReason.Message.ShouldContain("changed from 'sha256:old' to 'sha256:new'"); + + BuildReason supportReason = support.Reasons.Single( + reason => reason.Message.StartsWith("The image is required", StringComparison.Ordinal)); + supportReason.Message.ShouldContain("leaf/Dockerfile"); + supportReason.Cause.ShouldBe(leafReason); + } + + [TestMethod] + public async Task ChangedTagSetsRequireReuse() + { + using TempFolderContext tempFolder = TestHelper.UseTempFolder(); + ManifestInfo manifest = LoadManifest( + tempFolder, + CreateManifest( + CreateRepo("runtime", CreateImage( + ["old-shared", "new-shared"], + CreatePlatform( + CreateDockerfile("runtime", tempFolder, "base:tag"), + ["old", "new"]))))); + ImageArtifactDetails imageInfo = CreatePublishedImages( + manifest, + new Dictionary { ["runtime"] = "base@sha256:base" }); + ImageData image = imageInfo.Repos.Single().Images.Single(); + image.Platforms.Single().SimpleTags = ["old", "removed"]; + image.Manifest!.SharedTags = ["old-shared"]; + BuildGraph graph = BuildGraph.Create(manifest); + Mock manifestService = CreateDigestService("sha256:base"); + + IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + graph, + imageInfo, + CompositeBuildPolicy.ImageCache( + BuildAction.NoAction, + CreateBaseImageRule(manifest, manifestService.Object))); + + BuildPlanItem item = plan.ShouldHaveSingleItem(); + item.Action.ShouldBe(BuildAction.PublishExistingImage); + BuildReason reason = item.Reasons.Last(); + reason.Message.ShouldContain("[old, removed]"); + reason.Message.ShouldContain("[old, new]"); + reason.Message.ShouldContain("[old-shared, new-shared]"); + } + + [TestMethod] + public async Task InvalidatedSharedBuildForcesEveryTargetToBuild() + { + using TempFolderContext tempFolder = TestHelper.UseTempFolder(); + string dockerfile = CreateDockerfile("shared", tempFolder, "base:tag"); + ManifestInfo manifest = LoadManifest( + tempFolder, + CreateManifest( + CreateRepo("first", CreateImage(CreatePlatform(dockerfile, ["tag"]))), + CreateRepo("second", CreateImage(CreatePlatform(dockerfile, ["tag"]))))); + ImageArtifactDetails imageInfo = CreatePublishedImages( + manifest, + new Dictionary + { + ["first"] = "base@sha256:old", + ["second"] = "base@sha256:new", + }); + imageInfo.Repos.Single(repo => repo.Repo == "second") + .Images.Single().Platforms.Single().IsUnchanged = true; + BuildGraph graph = BuildGraph.Create(manifest); + Mock manifestService = CreateDigestService("sha256:new"); + + IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + graph, + imageInfo, + CompositeBuildPolicy.ImageCache( + BuildAction.NoAction, + CreateBaseImageRule(manifest, manifestService.Object))); + + BuildPlanItem first = GetItem(plan, "first"); + BuildPlanItem second = GetItem(plan, "second"); + first.Action.ShouldBe(BuildAction.BuildImage); + second.Action.ShouldBe(BuildAction.BuildImage); + BuildReason reason = second.Reasons.Last(); + reason.Message.ShouldContain("first"); + reason.Cause.ShouldNotBeNull().Message.ShouldContain("changed from"); + } + + [TestMethod] + public async Task SharedTagCreatesDependencyEdge() + { + using TempFolderContext tempFolder = TestHelper.UseTempFolder(); + ManifestInfo manifest = LoadManifest( + tempFolder, + CreateManifest( + CreateRepo("parent", CreateImage( + ["shared"], + CreatePlatform( + CreateDockerfile("parent", tempFolder, "base:tag"), + ["specific"]))), + CreateRepo("child", CreateImage(CreatePlatform( + CreateDockerfile("child", tempFolder, "parent:shared"), ["tag"]))))); + ImageArtifactDetails imageInfo = CreatePublishedImages( + manifest, + new Dictionary + { + ["parent"] = "base@sha256:old", + ["child"] = "parent@sha256:parent", + }); + BuildGraph graph = BuildGraph.Create(manifest); + Mock manifestService = CreateDigestService("sha256:new"); + + IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + graph, + imageInfo, + CompositeBuildPolicy.ImageCache( + BuildAction.NoAction, + CreateBaseImageRule(manifest, manifestService.Object))); + + BuildPlanItem child = GetItem(plan, "child"); + child.Action.ShouldBe(BuildAction.BuildImage); + child.Reasons.Last().Message.ShouldStartWith("Dependency"); + } + + [TestMethod] + public void StructurallyDifferentBuildArgumentsDoNotShareImages() + { + using TempFolderContext tempFolder = TestHelper.UseTempFolder(); + string dockerfile = CreateDockerfile("shared", tempFolder, "base:tag"); + Platform first = CreatePlatform(dockerfile, ["tag"]); + first.BuildArgs = new Dictionary { ["A"] = "x|B=y" }; + Platform second = CreatePlatform(dockerfile, ["tag"]); + second.BuildArgs = new Dictionary { ["A"] = "x", ["B"] = "y" }; + ManifestInfo manifest = LoadManifest( + tempFolder, + CreateManifest( + CreateRepo("first", CreateImage(first)), + CreateRepo("second", CreateImage(second)))); + BuildGraph graph = BuildGraph.Create(manifest); + + foreach (BuildTarget target in graph.Targets) + { + graph.SharedBuildTargets[target].ShouldHaveSingleItem(); + } + } + + [TestMethod] + public void DuplicateFromOverridesProduceOneTargetOverride() + { + using TempFolderContext tempFolder = TestHelper.UseTempFolder(); + const string FromImage = "mcr.microsoft.com/app:base"; + string dockerfile = CreateDockerfile("app", tempFolder, FromImage, FromImage); + Manifest model = CreateManifest( + CreateRepo("app", CreateImage(CreatePlatform(dockerfile, ["base"])))); + model.Registry = "mcr.microsoft.com"; + ManifestInfo manifest = LoadManifest(tempFolder, model, repoPrefix: "prefix/"); + BuildGraph graph = BuildGraph.Create(manifest); + + KeyValuePair imageOverride = graph.Targets.ShouldHaveSingleItem() + .FromImageOverrides.ShouldHaveSingleItem(); + imageOverride.Key.ShouldBe(FromImage); + imageOverride.Value.ShouldBe("mcr.microsoft.com/prefix/app:base"); + } + + [TestMethod] + public async Task CachedSiblingIsNotIncludedWhenAnotherChildBuilds() + { + using TempFolderContext tempFolder = TestHelper.UseTempFolder(); + ManifestInfo manifest = LoadManifest( + tempFolder, + CreateManifest( + CreateRepo("parent", CreateImage(CreatePlatform( + CreateDockerfile("parent", tempFolder, "base:tag"), ["tag"]))), + CreateRepo("first", CreateImage(CreatePlatform( + CreateDockerfile("first", tempFolder, "parent:tag"), ["tag"]))), + CreateRepo("second", CreateImage(CreatePlatform( + CreateDockerfile("second", tempFolder, "parent:tag"), ["tag"]))))); + ImageArtifactDetails imageInfo = CreatePublishedImages( + manifest, + new Dictionary + { + ["parent"] = "base@sha256:base", + ["first"] = "parent@sha256:parent", + ["second"] = "parent@sha256:parent", + }); + imageInfo.Repos.RemoveAll(repo => repo.Repo == "first"); + BuildGraph graph = BuildGraph.Create(manifest); + Mock manifestService = CreateDigestService("sha256:base"); + + IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + graph, + imageInfo, + CompositeBuildPolicy.ImageCache( + BuildAction.NoAction, + CreateBaseImageRule(manifest, manifestService.Object))); + + GetItem(plan, "parent").Action.ShouldBe(BuildAction.UsePublishedImage); + GetItem(plan, "first").Action.ShouldBe(BuildAction.BuildImage); + GetItem(plan, "second").Action.ShouldBe(BuildAction.NoAction); + } + + [TestMethod] + public async Task CustomRuleMethodCanAddAPlanningDecision() + { + using TempFolderContext tempFolder = TestHelper.UseTempFolder(); + ManifestInfo manifest = LoadManifest( + tempFolder, + CreateManifest( + CreateRepo("runtime", CreateImage(CreatePlatform( + CreateDockerfile("runtime", tempFolder, "base:tag"), ["tag"]))))); + BuildGraph graph = BuildGraph.Create(manifest); + IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + graph, + imageInfo: null, + new CompositeBuildPolicy( + BuildAction.NoAction, + new("No package changed."), + new PackageVersionChangedPolicy())); + + BuildPlanItem item = plan.ShouldHaveSingleItem(); + item.Action.ShouldBe(BuildAction.BuildImage); + item.Reasons.ShouldHaveSingleItem().Message.ShouldContain("openssl"); + } + + [TestMethod] + public async Task CompositePolicyAppliesEveryChildAndChoosesStrongestAction() + { + using TempFolderContext tempFolder = TestHelper.UseTempFolder(); + ManifestInfo manifest = LoadManifest( + tempFolder, + CreateManifest( + CreateRepo("runtime", CreateImage(CreatePlatform( + CreateDockerfile("runtime", tempFolder, "base:tag"), ["tag"]))))); + BuildGraph graph = BuildGraph.Create(manifest); + List appliedPolicies = []; + IBuildPolicy policy = new CompositeBuildPolicy( + BuildAction.NoAction, + new("No checks selected work."), + new TestPolicy( + appliedPolicies, + "use", + new( + BuildAction.UsePublishedImage, + new BuildReason("Use the published image."))), + new TestPolicy( + appliedPolicies, + "build", + new( + BuildAction.BuildImage, + new BuildReason("Build the image.")))); + + IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + graph, + CreatePublishedImages( + manifest, + new Dictionary { ["runtime"] = "base@sha256:base" }), + policy); + + appliedPolicies.ShouldBe(["use", "build"]); + BuildPlanItem item = plan.ShouldHaveSingleItem(); + item.Action.ShouldBe(BuildAction.BuildImage); + item.Reasons.Select(reason => reason.Message).ShouldBe( + ["Use the published image.", "Build the image."]); + } + + private static BuildPlanner CreatePlanner() => + new(Mock.Of>()); + + private static IBuildPolicy CreateBaseImageRule( + ManifestInfo manifest, + IManifestService manifestService) => + BaseImageChangedPolicy.FromRegistry( + new ImageDigestCache(new Lazy(() => manifestService)), + new ImageNameResolverForMatrix(new(), manifest, null, null), + isDryRun: false); + + private sealed class PackageVersionChangedPolicy : IBuildPolicy + { + public Task EvaluateAsync( + BuildPolicyContext context, + CancellationToken cancellationToken = default) => + Task.FromResult( + new BuildPolicyResult( + BuildAction.BuildImage, + new BuildReason("Package 'openssl' changed from '1.0' to '1.1'."))); + } + + private sealed class TestPolicy( + ICollection appliedPolicies, + string name, + BuildPolicyResult result) : IBuildPolicy + { + public Task EvaluateAsync( + BuildPolicyContext context, + CancellationToken cancellationToken = default) + { + appliedPolicies.Add(name); + return Task.FromResult(result); + } + } + + private static Mock CreateDigestService(string digest) + { + Mock manifestService = new(); + manifestService + .Setup(service => service.GetManifestDigestShaAsync(It.IsAny(), false)) + .ReturnsAsync(digest); + return manifestService; + } + + private static BuildPlanItem GetItem( + IReadOnlyList plan, + string repoName) => + plan.Single(item => item.Target.Repo.Name == repoName); + + private static ManifestInfo LoadManifest( + TempFolderContext tempFolder, + Manifest manifest, + string? repoPrefix = null) + { + string manifestPath = Path.Combine(tempFolder.Path, "manifest.json"); + File.WriteAllText(manifestPath, JsonConvert.SerializeObject(manifest)); + IManifestOptionsInfo options = GetManifestOptions(manifestPath); + Mock.Get(options) + .SetupGet(manifestOptions => manifestOptions.RepoPrefix) + .Returns(repoPrefix); + return TestHelper.CreateManifestJsonService().Load(options); + } + + private static ImageArtifactDetails CreatePublishedImages( + ManifestInfo manifest, + IReadOnlyDictionary baseImageDigests) + { + ImageArtifactDetails details = new(); + + foreach (RepoInfo repo in manifest.AllRepos) + { + RepoData repoData = new() { Repo = repo.Name }; + details.Repos.Add(repoData); + + foreach (ImageInfo image in repo.AllImages) + { + ImageData imageData = new() + { + Manifest = image.SharedTags.Any() + ? new ManifestData + { + SharedTags = image.SharedTags.Select(tag => tag.Name).ToList() + } + : null + }; + repoData.Images.Add(imageData); + + foreach (PlatformInfo platform in image.AllPlatforms) + { + imageData.Platforms.Add(new PlatformData(image, platform) + { + Dockerfile = platform.DockerfilePathRelativeToManifest, + Digest = $"{repo.Name}@sha256:{repo.Name}", + BaseImageDigest = baseImageDigests[repo.Name], + CommitUrl = $"https://example.test/{platform.DockerfilePathRelativeToManifest}", + SimpleTags = platform.Tags.Select(tag => tag.Name).ToList(), + }); + } + } + } + + return details; + } +} diff --git a/src/ImageBuilder.Tests/BuildCommandTests.cs b/src/ImageBuilder.Tests/BuildCommandTests.cs index c34c17119..ba048eb66 100644 --- a/src/ImageBuilder.Tests/BuildCommandTests.cs +++ b/src/ImageBuilder.Tests/BuildCommandTests.cs @@ -1,4 +1,4 @@ -#nullable disable +#nullable disable // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. @@ -10,6 +10,7 @@ using System.Threading.Tasks; using Azure.ResourceManager.ContainerRegistry.Models; using FluentAssertions; +using Microsoft.DotNet.ImageBuilder.Build; using Microsoft.DotNet.ImageBuilder.Commands; using Microsoft.DotNet.ImageBuilder.Configuration; using Microsoft.DotNet.ImageBuilder.Models.Image; @@ -136,7 +137,7 @@ public async Task BuildCommand_ImageInfoOutput_Basic() "1.0/runtime-deps/os", tempFolderContext, baseImageTag); string runtimeDockerfileRelativePath = DockerfileHelper.CreateDockerfile( - "1.0/runtime/os", tempFolderContext, $"{runtimeDepsRepo}:{tag}"); + "1.0/runtime/os", tempFolderContext, $"{runtimeDepsRepo}:shared"); string aspnetDockerfileRelativePath = DockerfileHelper.CreateDockerfile( "1.0/aspnet/os", tempFolderContext, $"{runtimeRepo}:{tag}"); @@ -159,7 +160,7 @@ public async Task BuildCommand_ImageInfoOutput_Basic() processService: processServiceMock.Object, copyImageService: Mock.Of(), manifestServiceFactory: manifestServiceFactoryMock.Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "image-info.json"); command.Options.IsPushEnabled = true; @@ -355,7 +356,7 @@ public async Task BuildCommand_ImageInfoOutput_DuplicatedPlatform() gitService: gitServiceMock.Object, copyImageService: Mock.Of(), manifestServiceFactory: manifestServiceFactoryMock.Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "image-info.json"); @@ -480,7 +481,7 @@ public async Task BuildCommand_Publish() dockerService: dockerServiceMock.Object, copyImageService: copyImageServiceMock.Object, manifestServiceFactory: CreateManifestServiceFactoryMock().Object, - imageCacheService: new ImageCacheService(Mock.Of>(), Mock.Of())); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.IsPushEnabled = true; @@ -554,7 +555,7 @@ public async Task BuildCommand_VerifyOnBaseImageArchMismatch() dockerService: dockerServiceMock.Object, copyImageService: Mock.Of(), manifestServiceFactory: CreateManifestServiceFactoryMock().Object, - imageCacheService: new ImageCacheService(Mock.Of>(), Mock.Of())); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); const string runtimeRelativeDir = "1.0/runtime/os"; @@ -608,7 +609,7 @@ public async Task BuildCommand_ArmVariantCompatibility(string manifestVariant, s dockerService: dockerServiceMock.Object, copyImageService: Mock.Of(), manifestServiceFactory: CreateManifestServiceFactoryMock().Object, - imageCacheService: new ImageCacheService(Mock.Of>(), Mock.Of())); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); const string runtimeRelativeDir = "1.0/runtime/os"; @@ -654,7 +655,7 @@ public async Task BuildCommand_BuildArgs() dockerService: dockerServiceMock.Object, copyImageService: Mock.Of(), manifestServiceFactory: CreateManifestServiceFactoryMock().Object, - imageCacheService: new ImageCacheService(Mock.Of>(), Mock.Of())); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.BuildArgs.Add("arg1", "val1"); command.Options.BuildArgs.Add("arg2", "val2a"); @@ -712,7 +713,7 @@ public async Task BuildCommand_DockerBuildOptions() dockerService: dockerServiceMock.Object, copyImageService: Mock.Of(), manifestServiceFactory: CreateManifestServiceFactoryMock().Object, - imageCacheService: new ImageCacheService(Mock.Of>(), Mock.Of())); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.DockerBuildOptions = ["--ulimit nofile=65536:65536", "--network host"]; @@ -764,7 +765,7 @@ public async Task BuildCommand_NoBaseImage_Build() BuildCommand command = CreateBuildCommand( dockerService: dockerServiceMock.Object, copyImageService: Mock.Of(), - imageCacheService: new ImageCacheService(Mock.Of>(), Mock.Of())); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.IsPushEnabled = true; @@ -861,7 +862,7 @@ public async Task BuildCommand_NoBaseImage_Cached() gitService: gitServiceMock.Object, copyImageService: copyImageServiceMock.Object, manifestServiceFactory: manifestServiceFactoryMock.Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "dest-image-info.json"); command.Options.ImageInfoSourcePath = Path.Combine(tempFolderContext.Path, "src-image-info.json"); @@ -1045,7 +1046,7 @@ public async Task BuildCommand_ImageInfoOutput_CustomDockerfile() gitService: gitServiceMock.Object, copyImageService: Mock.Of(), manifestServiceFactory: manifestServiceFactoryMock.Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "image-info.json"); command.Options.SourceRepoUrl = "https://source"; @@ -1089,7 +1090,7 @@ public async Task BuildCommand_ThrowsIfImageIsPulled(bool isSkipPullingEnabled) dockerService: dockerServiceMock.Object, copyImageService: Mock.Of(), manifestServiceFactory: CreateManifestServiceFactoryMock().Object, - imageCacheService: new ImageCacheService(Mock.Of>(), Mock.Of())); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.IsSkipPullingEnabled = isSkipPullingEnabled; @@ -1301,7 +1302,7 @@ public async Task BuildCommand_Caching( gitService: gitServiceMock.Object, copyImageService: copyImageServiceMock.Object, manifestServiceFactory: manifestServiceFactoryMock.Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "dest-image-info.json"); command.Options.ImageInfoSourcePath = Path.Combine(tempFolderContext.Path, "src-image-info.json"); @@ -1511,7 +1512,7 @@ public async Task BuildCommand_Caching_SharedDockerfile_MissingSourceImageInfoEn gitService: gitServiceMock.Object, copyImageService: copyImageServiceMock.Object, manifestServiceFactory: manifestServiceFactoryMock.Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "dest-image-info.json"); command.Options.ImageInfoSourcePath = Path.Combine(tempFolderContext.Path, "src-image-info.json"); @@ -1822,7 +1823,7 @@ public async Task BuildCommand_Caching_SharedDockerfile_MissingSourceImageInfoEn gitService: gitServiceMock.Object, copyImageService: copyImageServiceMock.Object, manifestServiceFactory: manifestServiceFactoryMock.Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "dest-image-info.json"); command.Options.ImageInfoSourcePath = Path.Combine(tempFolderContext.Path, "src-image-info.json"); @@ -2110,7 +2111,7 @@ public async Task BuildCommand_Caching_SharedDockerfile_NoExistingImageInfoEntri gitService: gitServiceMock.Object, copyImageService: copyImageServiceMock.Object, manifestServiceFactory: CreateManifestServiceFactoryMock(manifestServiceMock).Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "dest-image-info.json"); command.Options.ImageInfoSourcePath = Path.Combine(tempFolderContext.Path, "src-image-info.json"); @@ -2316,7 +2317,7 @@ public async Task BuildCommand_SharedDockerfile() gitService: gitServiceMock.Object, copyImageService: Mock.Of(), manifestServiceFactory: CreateManifestServiceFactoryMock(manifestServiceMock).Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "dest-image-info.json"); command.Options.ImageInfoSourcePath = Path.Combine(tempFolderContext.Path, "src-image-info.json"); @@ -2561,7 +2562,7 @@ public async Task BuildCommand_Caching_TagUpdate() gitService: gitServiceMock.Object, copyImageService: copyImageServiceMock.Object, manifestServiceFactory: CreateManifestServiceFactoryMock(manifestServiceMock).Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "dest-image-info.json"); command.Options.ImageInfoSourcePath = Path.Combine(tempFolderContext.Path, "src-image-info.json"); @@ -2783,7 +2784,7 @@ public async Task BuildCommand_Caching_SharedDockerfile_TagUpdate() gitService: gitServiceMock.Object, copyImageService: copyImageServiceMock.Object, manifestServiceFactory: CreateManifestServiceFactoryMock(manifestServiceMock).Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "dest-image-info.json"); command.Options.ImageInfoSourcePath = Path.Combine(tempFolderContext.Path, "src-image-info.json"); @@ -3090,7 +3091,7 @@ public async Task BuildCommand_MirroredImages(bool hasCachedImage, string srcBas gitService: gitServiceMock.Object, copyImageService: copyImageServiceMock.Object, manifestServiceFactory: CreateManifestServiceFactoryMock(manifestServiceMock).Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "image-info.json"); command.Options.ImageInfoSourcePath = Path.Combine(tempFolderContext.Path, "src-image-info.json"); @@ -3433,7 +3434,7 @@ public async Task BuildCommand_MirroredImages_External(string baseImageRegistry, gitService: gitServiceMock.Object, copyImageService: Mock.Of(), manifestServiceFactory: CreateManifestServiceFactoryMock(manifestServiceMock).Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "image-info.json"); command.Options.IsPushEnabled = true; @@ -3529,7 +3530,7 @@ public async Task BuildCommand_MirroredImages_BaseImageTagOverride() gitService: gitServiceMock.Object, copyImageService: Mock.Of(), manifestServiceFactory: CreateManifestServiceFactoryMock(manifestServiceMock).Object, - imageCacheService: new ImageCacheService(Mock.Of>(), gitServiceMock.Object)); + buildPlanner: CreateBuildPlanner()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.ImageInfoOutputPath = Path.Combine(tempFolderContext.Path, "image-info.json"); command.Options.IsPushEnabled = true; @@ -3641,7 +3642,7 @@ private static BuildCommand CreateBuildCommand( IManifestServiceFactory? manifestServiceFactory = null, IRegistryCredentialsProvider? registryCredentialsProvider = null, IAzureTokenCredentialProvider? azureTokenCredentialProvider = null, - IImageCacheService? imageCacheService = null) + BuildPlanner? buildPlanner = null) { BuildCommand command = new( manifestJsonService ?? TestHelper.CreateManifestJsonService(), @@ -3653,10 +3654,14 @@ private static BuildCommand CreateBuildCommand( manifestServiceFactory ?? Mock.Of(), registryCredentialsProvider ?? Mock.Of(), azureTokenCredentialProvider ?? Mock.Of(), - imageCacheService ?? Mock.Of()); + buildPlanner ?? CreateBuildPlanner()); return command; } + + private static BuildPlanner CreateBuildPlanner() => + new(Mock.Of>()); + #nullable disable private static Mock CreateDockerServiceMock(string buildOutput = null) diff --git a/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs b/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs index 68f1c0130..3d3306373 100644 --- a/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs +++ b/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs @@ -8,7 +8,9 @@ using System.IO; using System.Linq; using System.Net.Http; +using System.Threading; using System.Threading.Tasks; +using Microsoft.DotNet.ImageBuilder.Build; using Microsoft.DotNet.ImageBuilder.Commands; using Microsoft.DotNet.ImageBuilder.Models.Image; using Microsoft.DotNet.ImageBuilder.Models.Manifest; @@ -159,56 +161,82 @@ public async Task GenerateBuildMatrixCommand_PlatformDependencyGraph(string filt } } - private static void SetCacheResult(Mock imageCacheServiceMock, string dockerfilePath, ImageCacheState cacheState) + [TestMethod] + public async Task PlatformDependencyGraph_SharedTagDependency() { - imageCacheServiceMock - .Setup(o => o.CheckForCachedImageAsync( - It.IsAny(), - It.Is(platform => platform.Dockerfile == dockerfilePath), - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny())) - .ReturnsAsync(new ImageCacheResult(cacheState, false, null)); + using TempFolderContext tempFolder = TestHelper.UseTempFolder(); + string parentDockerfile = CreateDockerfile( + "parent", + tempFolder, + "base:tag"); + string childDockerfile = CreateDockerfile( + "child", + tempFolder, + "parent:shared"); + Manifest manifest = CreateManifest( + CreateRepo( + "parent", + CreateImage( + ["shared"], + CreatePlatform(parentDockerfile, ["specific"]))), + CreateRepo( + "child", + CreateImage(CreatePlatform(childDockerfile, ["tag"])))); + GenerateBuildMatrixCommand command = CreateCommand(); + command.Options.Manifest = Path.Combine(tempFolder.Path, "manifest.json"); + command.Options.MatrixType = MatrixType.PlatformDependencyGraph; + File.WriteAllText( + command.Options.Manifest, + JsonConvert.SerializeObject(manifest)); + + command.LoadManifest(); + BuildMatrixInfo matrix = (await command.GenerateMatrixInfoAsync()) + .ShouldHaveSingleItem(); + BuildLegInfo leg = matrix.Legs.ShouldHaveSingleItem(); + + leg.Variables + .Single(variable => variable.Name == "imageBuilderPaths") + .Value + .ShouldBe("--path parent/Dockerfile --path child/Dockerfile"); } [TestMethod] [DataRow( - ImageCacheState.NotCached, - ImageCacheState.NotCached, + BuildAction.BuildImage, + BuildAction.BuildImage, "--path 1.0/runtime/os/amd64/Dockerfile --path 1.0/sdk/os/amd64/Dockerfile", "--path 2.0/runtime/os/amd64/Dockerfile --path 2.0/sdk/os/amd64/Dockerfile")] [DataRow( - ImageCacheState.Cached, - ImageCacheState.Cached, + BuildAction.NoAction, + BuildAction.NoAction, "--path 2.0/runtime/os/amd64/Dockerfile --path 2.0/sdk/os/amd64/Dockerfile")] [DataRow( - ImageCacheState.Cached, - ImageCacheState.Cached, + BuildAction.NoAction, + BuildAction.NoAction, "--path 1.0/standalone/os/amd64/Dockerfile", "--path 2.0/standalone/os/amd64/Dockerfile", null, "*standalone*")] [DataRow( - ImageCacheState.CachedWithMissingTags, - ImageCacheState.Cached, + BuildAction.UsePublishedImage, + BuildAction.NoAction, + "--path 1.0/runtime/os/amd64/Dockerfile", "--path 2.0/runtime/os/amd64/Dockerfile --path 2.0/sdk/os/amd64/Dockerfile")] [DataRow( - ImageCacheState.Cached, - ImageCacheState.NotCached, + BuildAction.UsePublishedImage, + BuildAction.BuildImage, "--path 1.0/runtime/os/amd64/Dockerfile --path 1.0/sdk/os/amd64/Dockerfile", "--path 2.0/runtime/os/amd64/Dockerfile --path 2.0/sdk/os/amd64/Dockerfile")] [DataRow( - ImageCacheState.NotCached, - ImageCacheState.NotCached, + BuildAction.BuildImage, + BuildAction.BuildImage, "--path 1.0/runtime/os/amd64/Dockerfile --path 1.0/sdk/os/amd64/Dockerfile", "--path 1.0/standalone/os/amd64/Dockerfile", "--path 2.0/runtime/os/amd64/Dockerfile --path 2.0/sdk/os/amd64/Dockerfile", "")] // Clear out the path filters to ensure all images are included public async Task FilterOutCachedImages( - ImageCacheState runtime1CacheState, - ImageCacheState sdk1CacheState, + BuildAction runtime1Action, + BuildAction sdk1Action, string leg1ExpectedPaths, string leg2ExpectedPaths = null, string leg3ExpectedPaths = null, @@ -251,14 +279,43 @@ public async Task FilterOutCachedImages( CreatePlatform(dockerfileSdk2Path = CreateDockerfile(Sdk2RelativeDir, tempFolderContext, "runtime:2.0"), ["2.0"]))) ); - Mock imageCacheServiceMock = new(); - SetCacheResult(imageCacheServiceMock, dockerfileStandalone1Path, ImageCacheState.NotCached); - SetCacheResult(imageCacheServiceMock, dockerfileRuntime1Path, runtime1CacheState); - SetCacheResult(imageCacheServiceMock, dockerfileSdk1Path, sdk1CacheState); - SetCacheResult(imageCacheServiceMock, dockerfileRuntime2Path, ImageCacheState.NotCached); - SetCacheResult(imageCacheServiceMock, dockerfileSdk2Path, ImageCacheState.NotCached); + Dictionary actions = new() + { + [dockerfileRuntime1Path] = runtime1Action, + [dockerfileSdk1Path] = sdk1Action, + }; + Mock buildPlannerMock = new( + Mock.Of>()); + buildPlannerMock + .Setup(planner => planner.CreatePlanAsync( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(( + BuildGraph graph, + ImageArtifactDetails imageInfo, + IBuildPolicy policy, + CancellationToken _) => Task.FromResult( + (IReadOnlyList)graph.Targets.Select(target => + new BuildPlanItem( + target, + actions.GetValueOrDefault( + target.Platform.DockerfilePathRelativeToManifest, + BuildAction.BuildImage), + Reasons: + [ + new("Test-selected action.") + ], + PublishedImage: null)) + .ToArray())); - GenerateBuildMatrixCommand command = new(TestHelper.CreateManifestJsonService(), imageCacheServiceMock.Object, Mock.Of(), Mock.Of>()); + GenerateBuildMatrixCommand command = new( + TestHelper.CreateManifestJsonService(), + buildPlannerMock.Object, + Mock.Of(), + Mock.Of(), + Mock.Of>()); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); command.Options.MatrixType = MatrixType.PlatformDependencyGraph; command.Options.ImageInfoPath = Path.Combine(tempFolderContext.Path, "imageinfo.json"); @@ -1707,13 +1764,12 @@ private static GenerateBuildMatrixCommand SetupTrimCacheTest( externalImageDigestResults: externalImageDigestResults ?? []); } - ImageCacheService imageCacheService = new( - Mock.Of>(), - gitServiceMock.Object); + BuildPlanner buildPlanner = new(Mock.Of>()); GenerateBuildMatrixCommand command = new( TestHelper.CreateManifestJsonService(), - imageCacheService, + buildPlanner, + gitServiceMock.Object, manifestServiceFactoryMock.Object, Mock.Of>()); @@ -1758,6 +1814,11 @@ private static GenerateBuildMatrixCommand SetupTrimCacheTest( } private static GenerateBuildMatrixCommand CreateCommand() => - new(TestHelper.CreateManifestJsonService(), Mock.Of(), Mock.Of(), Mock.Of>()); + new( + TestHelper.CreateManifestJsonService(), + new BuildPlanner(Mock.Of>()), + Mock.Of(), + Mock.Of(), + Mock.Of>()); } } diff --git a/src/ImageBuilder.Tests/GetStaleImagesCommandTests.cs b/src/ImageBuilder.Tests/GetStaleImagesCommandTests.cs index 67836b594..f1fe3e433 100644 --- a/src/ImageBuilder.Tests/GetStaleImagesCommandTests.cs +++ b/src/ImageBuilder.Tests/GetStaleImagesCommandTests.cs @@ -1,4 +1,4 @@ -#nullable disable +#nullable disable // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. @@ -12,6 +12,7 @@ using System.Text; using System.Threading.Tasks; using LibGit2Sharp; +using Microsoft.DotNet.ImageBuilder.Build; using Microsoft.DotNet.ImageBuilder.Commands; using Microsoft.DotNet.ImageBuilder.Models.Image; using Microsoft.DotNet.ImageBuilder.Models.Manifest; @@ -1373,6 +1374,7 @@ public async Task GetStaleImagesCommand_InternalFromOnly() const string repo1 = "test-repo"; const string dockerfile1Path = "dockerfile1/Dockerfile"; const string dockerfile2Path = "dockerfile2/Dockerfile"; + string parentDigest = $"sha256:{new string('0', 64)}"; SubscriptionInfo[] subscriptionInfos = new SubscriptionInfo[] { @@ -1398,11 +1400,15 @@ public async Task GetStaleImagesCommand_InternalFromOnly() { Platforms = { - CreatePlatform(dockerfile1Path), + CreatePlatform( + dockerfile1Path, + baseImageDigest: $"{repo1}@{parentDigest}", + simpleTags: new List { "tag1" }), CreatePlatform( dockerfile2Path, + digest: parentDigest, baseImageDigest: "base1@base1digest", - simpleTags: new List { "tag1" }) + simpleTags: new List { "tag2" }) } } } @@ -1662,6 +1668,7 @@ private static Subscription CreateSubscription( /// private class TestFixture : IDisposable { + private const string DockerfileCommitSha = "current-commit"; private readonly List filesToCleanup = new List(); private readonly List foldersToCleanup = new List(); private readonly Dictionary imageDigests = new Dictionary(); @@ -1694,6 +1701,21 @@ public TestFixture( string osType = "*") { this.osType = osType; + foreach (SubscriptionInfo subscriptionInfo in subscriptionInfos) + { + string sourceRepoUrl = + $"https://github.com/{subscriptionInfo.Subscription.Manifest.Owner}/" + + subscriptionInfo.Subscription.Manifest.Repo; + foreach (PlatformData platform in subscriptionInfo.ImageInfo.Repos + .SelectMany(repo => repo.Images) + .SelectMany(image => image.Platforms)) + { + platform.CommitUrl = + $"{sourceRepoUrl}/blob/{DockerfileCommitSha}/" + + PathHelper.NormalizePath(platform.Dockerfile); + } + } + this.subscriptionsPath = this.SerializeJsonObjectToTempFile( subscriptionInfos.Select(tuple => tuple.Subscription).ToArray()); @@ -1754,7 +1776,7 @@ public void Verify(IDictionary> expectedPathsBySubsc string[] actualPaths = pathsBySubscription .First(imagePaths => imagePaths.SubscriptionId == kvp.Key.Id).ImagePaths; - actualPaths.ShouldBe(kvp.Value); + actualPaths.ShouldBe(kvp.Value, ignoreOrder: true); } } @@ -1769,7 +1791,12 @@ private string SerializeJsonObjectToTempFile(object jsonObject) private GetStaleImagesCommand CreateCommand() { GetStaleImagesCommand command = new( - this.ManifestServiceFactoryMock.Object, TestHelper.CreateManifestJsonService(), this.loggerServiceMock.Object, this.octokitClientFactory, this.gitService); + this.ManifestServiceFactoryMock.Object, + TestHelper.CreateManifestJsonService(), + this.loggerServiceMock.Object, + this.octokitClientFactory, + this.gitService, + new BuildPlanner(Mock.Of>())); command.Options.SubscriptionOptions.SubscriptionsPath = this.subscriptionsPath; command.Options.VariableName = VariableName; command.Options.FilterOptions.Platform.OsType = this.osType; @@ -1854,6 +1881,11 @@ private IGitService CreateGitService( Dictionary> dockerfileInfos) { Mock gitServiceMock = new(); + gitServiceMock + .Setup(service => service.GetCommitSha( + It.IsAny(), + useFullHash: true)) + .Returns(DockerfileCommitSha); foreach (SubscriptionInfo subscriptionInfo in subscriptionInfos) { diff --git a/src/ImageBuilder/Build/BuildGraph.cs b/src/ImageBuilder/Build/BuildGraph.cs new file mode 100644 index 000000000..71a5cce03 --- /dev/null +++ b/src/ImageBuilder/Build/BuildGraph.cs @@ -0,0 +1,195 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.Json; +using Microsoft.DotNet.ImageBuilder.Models.Image; +using Microsoft.DotNet.ImageBuilder.ViewModel; + +namespace Microsoft.DotNet.ImageBuilder.Build; + +/// +/// A current image definition that ImageBuilder can build. +/// +public sealed record BuildTarget( + RepoInfo Repo, + ImageInfo Image, + PlatformInfo Platform, + IReadOnlyDictionary FromImageOverrides); + +/// +/// Dependency, shared-build, and published-image data for build targets. +/// +public sealed record BuildGraph( + IReadOnlyList Targets, + IReadOnlyDictionary> Parents, + IReadOnlyDictionary> Children, + IReadOnlyDictionary> SharedBuildTargets) +{ + /// + /// Creates a graph for every platform in the manifest. + /// + public static BuildGraph Create(ManifestInfo manifest) => + CreateForPlatforms(manifest, manifest.GetAllPlatforms()); + + /// + /// Creates a graph for the manifest platforms selected by its command-line filters. + /// + public static BuildGraph CreateFiltered(ManifestInfo manifest) => + CreateForPlatforms(manifest, manifest.GetFilteredPlatforms()); + + /// + /// Creates a graph for selected platforms after applying the manifest's command-line filters. + /// + public static BuildGraph CreateFiltered( + ManifestInfo manifest, + Func include) => + CreateForPlatforms(manifest, manifest.GetFilteredPlatforms().Where(include)); + + private static BuildGraph CreateForPlatforms( + ManifestInfo manifest, + IEnumerable platforms) + { + // Create build targets. These contain all the information needed to build an image. + HashSet platformSet = platforms.ToHashSet(); + BuildTarget[] targets = manifest.AllRepos + .SelectMany(repo => repo.AllImages.SelectMany(image => + image.AllPlatforms + .Where(platformSet.Contains) + .Select(platform => new BuildTarget( + repo, + image, + platform, + GetFromImageOverrides(manifest, platform))))) + .ToArray(); + + // Index every platform and shared tag so internal FROM references can be resolved. + Dictionary> targetsByTag = []; + foreach (BuildTarget target in targets) + { + foreach (string tag in GetTags(target)) + { + if (!targetsByTag.TryGetValue(tag, out List? taggedTargets)) + { + taggedTargets = []; + targetsByTag.Add(tag, taggedTargets); + } + + taggedTargets.Add(target); + } + } + + // Create the dependency graph from internal FROM references. + Dictionary> parents = targets.ToDictionary( + target => target, + target => (IReadOnlyList)target.Platform.InternalFromImages + .Select(fromImage => ResolveParent(target, fromImage, targetsByTag)) + .Where(parent => parent is not null) + .Cast() + .Distinct() + .ToArray()); + Dictionary> mutableChildren = targets.ToDictionary( + target => target, + _ => new List()); + + foreach ((BuildTarget child, IReadOnlyList targetParents) in parents) + { + foreach (BuildTarget parent in targetParents) + { + mutableChildren[parent].Add(child); + } + } + + Dictionary> children = + mutableChildren.ToDictionary( + pair => pair.Key, + pair => (IReadOnlyList)pair.Value); + + // Group targets that produce equivalent image content and can share published images. + Dictionary> sharedBuildTargets = []; + foreach (IGrouping group in targets.GroupBy(GetSharedBuildKey)) + { + BuildTarget[] groupTargets = group.ToArray(); + foreach (BuildTarget target in groupTargets) + { + sharedBuildTargets[target] = groupTargets; + } + } + + return new BuildGraph( + targets, + parents, + children, + sharedBuildTargets); + } + + private static IReadOnlyDictionary GetFromImageOverrides( + ManifestInfo manifest, + PlatformInfo platform) => + platform.OverriddenFromImages + .Distinct(StringComparer.Ordinal) + .ToDictionary( + fromImage => fromImage, + fromImage => + { + string fromRepo = DockerHelper.GetRepo(fromImage); + RepoInfo repo = manifest.AllRepos.First(repo => + repo.FullModelName == fromRepo); + return DockerHelper.ReplaceRepo(fromImage, repo.QualifiedName); + }); + + private static IEnumerable GetTags(BuildTarget target) => + target.Platform.Tags + .Concat(target.Image.SharedTags) + .Select(tag => tag.FullyQualifiedName) + .Distinct(StringComparer.Ordinal); + + private static BuildTarget? ResolveParent( + BuildTarget child, + string fromImage, + IReadOnlyDictionary> targetsByTag) + { + if (!targetsByTag.TryGetValue(fromImage, out List? candidates)) + { + return null; + } + + BuildTarget[] distinctCandidates = candidates.Distinct().ToArray(); + BuildTarget[] platformMatches = distinctCandidates + .Where(candidate => HasSameTargetPlatform(candidate, child)) + .ToArray(); + BuildTarget[] matches = platformMatches.Length > 0 + ? platformMatches + : distinctCandidates; + + return matches.Length switch + { + 1 => matches[0], + _ => throw new InvalidOperationException( + $"Internal image '{fromImage}' has {matches.Length} candidate platforms for " + + $"'{child.Platform.DockerfilePathRelativeToManifest}'.") + }; + } + + private static bool HasSameTargetPlatform( + BuildTarget first, + BuildTarget second) => + first.Platform.Model.OS == second.Platform.Model.OS && + first.Platform.Model.OsVersion == second.Platform.Model.OsVersion && + first.Platform.Model.Architecture == second.Platform.Model.Architecture && + first.Platform.Model.Variant == second.Platform.Model.Variant; + + private static string GetSharedBuildKey(BuildTarget target) => + JsonSerializer.Serialize(new + { + target.Platform.DockerfilePathRelativeToManifest, + target.Platform.PlatformLabel, + BuildArgs = target.Platform.BuildArgs + .OrderBy(argument => argument.Key, StringComparer.Ordinal), + FromImageOverrides = target.FromImageOverrides + .OrderBy(image => image.Key, StringComparer.Ordinal) + }); +} diff --git a/src/ImageBuilder/Build/BuildPlan.cs b/src/ImageBuilder/Build/BuildPlan.cs new file mode 100644 index 000000000..ff817248b --- /dev/null +++ b/src/ImageBuilder/Build/BuildPlan.cs @@ -0,0 +1,60 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Collections.Generic; +using Microsoft.DotNet.ImageBuilder.Models.Image; + +namespace Microsoft.DotNet.ImageBuilder.Build; + +/// +/// Work that ImageBuilder must perform for a target. +/// +public enum BuildAction +{ + /// + /// The published image is valid and this invocation does not need it locally. + /// + NoAction = 0, + + /// + /// Use the valid published image without running a Docker build. The image may need to be + /// pulled, imported, or retagged for this invocation. + /// + UsePublishedImage = 1, + + /// + /// Use the valid published image and continue it through downstream processing because its + /// published metadata, such as tags, must be updated. + /// + PublishExistingImage = 2, + + /// + /// Run a Docker build for the target. + /// + BuildImage = 3 +} + +/// +/// An explanation for a planned action, optionally linked to the reason that caused it. +/// +public sealed record BuildReason( + string Message, + BuildReason? Cause = null); + +/// +/// Published image-info associated with a build target. +/// +/// Target whose image-info supplied the data. +/// Existing published platform data. +/// Published image-level tags stored outside . +public sealed record PublishedImage( + BuildTarget Source, + PlatformData Image, + IReadOnlyList SharedTags); + +public sealed record BuildPlanItem( + BuildTarget Target, + BuildAction Action, + IReadOnlyList Reasons, + PublishedImage? PublishedImage); diff --git a/src/ImageBuilder/Build/BuildPlanner.cs b/src/ImageBuilder/Build/BuildPlanner.cs new file mode 100644 index 000000000..aa7708776 --- /dev/null +++ b/src/ImageBuilder/Build/BuildPlanner.cs @@ -0,0 +1,344 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.DotNet.ImageBuilder.Models.Image; +using Microsoft.DotNet.ImageBuilder.ViewModel; + +namespace Microsoft.DotNet.ImageBuilder.Build; + +/// +/// Calculates the image work required for a build without executing that work. +/// +public class BuildPlanner(ILogger logger) +{ + private readonly ILogger _logger = + logger ?? throw new ArgumentNullException(nameof(logger)); + + public virtual async Task> CreatePlanAsync( + BuildGraph graph, + ImageArtifactDetails? imageInfo, + IBuildPolicy policy, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(graph); + ArgumentNullException.ThrowIfNull(policy); + + Dictionary publishedImages = + CreatePublishedImageIndex(graph, imageInfo); + + Dictionary items = []; + + // Shared builds act as one node. Evaluate and apply each node from roots to leaves so + // every parent decision is final before its children are considered. + foreach (IReadOnlyList sharedBuild in + GetSharedBuildsInDependencyOrder(graph)) + { + foreach (BuildTarget target in sharedBuild) + { + items.Add( + target, + await EvaluateAsync( + graph, + target, + publishedImages, + policy, + cancellationToken)); + } + + PropagateBuildsFromParents(graph, sharedBuild, items); + UnifySharedBuildActions(sharedBuild, items); + } + + // Built images need their direct internal parents available locally. An unchanged parent + // can use its published image; it does not need its own parents because it is not rebuilt. + UsePublishedParentsForBuilds(graph, items, publishedImages); + + BuildPlanItem[] plan = graph.Targets + .Select(target => items[target]) + .ToArray(); + LogPlan(plan); + return plan; + } + + private static Dictionary CreatePublishedImageIndex( + BuildGraph graph, + ImageArtifactDetails? imageInfo) + { + Dictionary targetsByPlatform = graph.Targets.ToDictionary( + target => target.Platform); + Dictionary publishedImages = imageInfo?.Repos + .SelectMany(repo => repo.Images) + .SelectMany(image => image.Platforms.Select(platform => + ( + Platform: platform, + SharedTags: (IReadOnlyList) + (image.Manifest?.SharedTags?.ToArray() ?? [])))) + .Where(item => + item.Platform.PlatformInfo is not null && + targetsByPlatform.ContainsKey(item.Platform.PlatformInfo)) + .GroupBy(item => item.Platform.PlatformInfo!) + .ToDictionary( + group => targetsByPlatform[group.Key], + group => + { + var item = group.First(); + BuildTarget target = targetsByPlatform[group.Key]; + return new PublishedImage( + target, + item.Platform, + item.SharedTags); + }) + ?? []; + + // Equivalent targets can reuse one another's published image when only one target has + // a direct image-info entry. + foreach (IReadOnlyList sharedBuild in + graph.SharedBuildTargets.Values.Distinct()) + { + BuildTarget? source = sharedBuild.FirstOrDefault(publishedImages.ContainsKey); + if (source is null) + { + continue; + } + + foreach (BuildTarget target in sharedBuild) + { + publishedImages.TryAdd( + target, + new PublishedImage( + source, + publishedImages[source].Image, + publishedImages[source].SharedTags)); + } + } + + return publishedImages; + } + + private static async Task EvaluateAsync( + BuildGraph graph, + BuildTarget target, + IReadOnlyDictionary publishedImages, + IBuildPolicy policy, + CancellationToken cancellationToken) + { + bool hasPublishedImage = publishedImages.TryGetValue( + target, + out PublishedImage? publishedImage); + List reasons = []; + if (publishedImage is not null && publishedImage.Source != target) + { + reasons.Add(new( + $"Published image metadata is shared with '{GetName(publishedImage.Source)}'.")); + } + + BuildPolicyResult result = await policy.EvaluateAsync( + new(graph, target, publishedImages), + cancellationToken); + reasons.AddRange(result.Reasons); + return CreateItem( + target, + result.Action, + reasons, + hasPublishedImage ? publishedImage : null); + } + + private static IReadOnlyList> + GetSharedBuildsInDependencyOrder(BuildGraph graph) + { + IReadOnlyList> sharedBuilds = graph.SharedBuildTargets + .Values + .DistinctBy(targets => targets[0]) + .ToArray(); + Dictionary> sharedBuildByTarget = + sharedBuilds + .SelectMany(sharedBuild => sharedBuild.Select( + target => (Target: target, SharedBuild: sharedBuild))) + .ToDictionary(item => item.Target, item => item.SharedBuild); + List> ordered = []; + HashSet visiting = []; + HashSet visited = []; + + void Visit(IReadOnlyList sharedBuild) + { + BuildTarget key = sharedBuild[0]; + if (visited.Contains(key)) + { + return; + } + + if (!visiting.Add(key)) + { + throw new InvalidOperationException( + $"Build dependency cycle detected at '{GetName(key)}'."); + } + + foreach (BuildTarget parent in sharedBuild + .SelectMany(target => graph.Parents[target]) + .Distinct()) + { + IReadOnlyList parentBuild = sharedBuildByTarget[parent]; + if (parentBuild[0] != key) + { + Visit(parentBuild); + } + } + + visiting.Remove(key); + visited.Add(key); + ordered.Add(sharedBuild); + } + + foreach (IReadOnlyList sharedBuild in sharedBuilds) + { + Visit(sharedBuild); + } + + return ordered; + } + + private static void PropagateBuildsFromParents( + BuildGraph graph, + IReadOnlyList sharedBuild, + IDictionary items) + { + foreach (BuildTarget target in sharedBuild) + { + foreach (BuildTarget parent in graph.Parents[target] + .Where(parent => items[parent].Action == BuildAction.BuildImage)) + { + BuildPlanItem item = items[target]; + BuildReason reason = new( + $"Dependency '{GetName(parent)}' must build.", + GetCause(items[parent])); + items[target] = item with + { + Action = BuildAction.BuildImage, + Reasons = item.Reasons.Contains(reason) + ? item.Reasons + : [..item.Reasons, reason] + }; + } + } + } + + private static void UnifySharedBuildActions( + IReadOnlyList sharedBuild, + IDictionary items) + { + BuildPlanItem? invalidatedItem = sharedBuild + .Select(target => items[target]) + .FirstOrDefault(item => item.Action == BuildAction.BuildImage); + if (invalidatedItem is null) + { + return; + } + + foreach (BuildTarget target in sharedBuild) + { + BuildPlanItem item = items[target]; + if (item.Action == BuildAction.BuildImage) + { + continue; + } + + items[target] = item with + { + Action = BuildAction.BuildImage, + Reasons = + [ + ..item.Reasons, + new( + $"Equivalent target '{GetName(invalidatedItem.Target)}' must build.", + GetCause(invalidatedItem)) + ] + }; + } + } + + private static void UsePublishedParentsForBuilds( + BuildGraph graph, + IDictionary items, + IReadOnlyDictionary publishedImages) + { + foreach (BuildPlanItem childItem in items.Values + .Where(item => item.Action == BuildAction.BuildImage) + .ToArray()) + { + foreach (BuildTarget parent in graph.Parents[childItem.Target]) + { + BuildPlanItem parentItem = items[parent]; + BuildReason reason = new( + $"The image is required by '{GetName(childItem.Target)}'.", + GetCause(childItem)); + + if (parentItem.Action == BuildAction.NoAction) + { + if (!publishedImages.ContainsKey(parent)) + { + throw new InvalidOperationException( + $"Required dependency '{GetName(parent)}' has no published image."); + } + + items[parent] = parentItem with + { + Action = BuildAction.UsePublishedImage, + Reasons = [reason, ..parentItem.Reasons] + }; + } + else if (!parentItem.Reasons.Contains(reason)) + { + items[parent] = parentItem with + { + Reasons = [reason, ..parentItem.Reasons] + }; + } + } + } + } + + private static BuildPlanItem CreateItem( + BuildTarget target, + BuildAction action, + IReadOnlyList reasons, + PublishedImage? publishedImage) + { + if ((action is BuildAction.UsePublishedImage or BuildAction.PublishExistingImage) && + publishedImage is null) + { + throw new InvalidOperationException( + $"Planning selected '{action}' for '{GetName(target)}' " + + "without a published image."); + } + + return new(target, action, reasons, publishedImage); + } + + private void LogPlan(IReadOnlyList plan) + { + foreach (BuildPlanItem item in plan) + { + _logger.LogInformation( + "Build plan for {DockerfilePath}: {Action}. {Reasons}", + GetName(item.Target), + item.Action, + string.Join(" ", item.Reasons.Select(FormatReason))); + } + } + + private static BuildReason GetCause(BuildPlanItem item) => item.Reasons.Last(); + + private static string FormatReason(BuildReason reason) => + reason.Cause is null + ? reason.Message + : $"{reason.Message} {FormatReason(reason.Cause)}"; + + private static string GetName(BuildTarget target) => + $"{target.Repo.Name} ({target.Platform.DockerfilePathRelativeToManifest})"; +} diff --git a/src/ImageBuilder/Build/BuildPolicies.cs b/src/ImageBuilder/Build/BuildPolicies.cs new file mode 100644 index 000000000..53a506d0d --- /dev/null +++ b/src/ImageBuilder/Build/BuildPolicies.cs @@ -0,0 +1,316 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; +using Azure; +using Microsoft.DotNet.ImageBuilder.Models.Image; + +namespace Microsoft.DotNet.ImageBuilder.Build; + +public sealed record BuildPolicyContext( + BuildGraph Graph, + BuildTarget Target, + IReadOnlyDictionary PublishedImages); + +public sealed record BuildPolicyResult( + BuildAction Action, + IReadOnlyList Reasons) +{ + public BuildPolicyResult( + BuildAction action, + BuildReason reason) + : this(action, [reason]) + { + } + + public static BuildPolicyResult None { get; } = new(BuildAction.NoAction, []); +} + +/// +/// Evaluates one aspect of the work required for a build target. +/// +public interface IBuildPolicy +{ + Task EvaluateAsync( + BuildPolicyContext context, + CancellationToken cancellationToken = default); +} + +/// +/// Applies every child policy and combines their results into one decision. The action with the +/// highest value wins. +/// +public sealed class CompositeBuildPolicy( + BuildAction defaultAction, + BuildReason defaultReason, + params IReadOnlyList policies) : IBuildPolicy +{ + public static CompositeBuildPolicy ImageCache( + BuildAction validImageAction, + params IReadOnlyList checks) => + new( + validImageAction, + new( + validImageAction == BuildAction.UsePublishedImage + ? "All checks passed, so this invocation will use the published image." + : "All checks passed, so no work is required."), + [ + new MissingPublishedImagePolicy(), + ..checks, + new TagSetChangedPolicy() + ]); + + public async Task EvaluateAsync( + BuildPolicyContext context, + CancellationToken cancellationToken = default) + { + BuildPolicyResult[] results = await Task.WhenAll( + policies.Select(policy => policy.EvaluateAsync(context, cancellationToken))); + BuildAction childAction = results + .Select(result => result.Action) + .DefaultIfEmpty(BuildAction.NoAction) + .Max(); + BuildAction action = (BuildAction)Math.Max((int)childAction, (int)defaultAction); + IReadOnlyList reasons = results + .SelectMany(result => result.Reasons) + .ToArray(); + + return new( + action, + childAction == BuildAction.NoAction ? [..reasons, defaultReason] : reasons); + } +} + +public sealed class AlwaysBuildPolicy(string reason = "Caching is disabled.") : IBuildPolicy +{ + public Task EvaluateAsync( + BuildPolicyContext context, + CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + return Task.FromResult( + new BuildPolicyResult( + BuildAction.BuildImage, + new BuildReason(reason))); + } +} + +public sealed class MissingPublishedImagePolicy : IBuildPolicy +{ + public Task EvaluateAsync( + BuildPolicyContext context, + CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + BuildPolicyResult result = !context.PublishedImages.ContainsKey(context.Target) + ? new( + BuildAction.BuildImage, + new BuildReason("No published image metadata exists.")) + : BuildPolicyResult.None; + return Task.FromResult(result); + } +} + +public sealed class TagSetChangedPolicy : IBuildPolicy +{ + public Task EvaluateAsync( + BuildPolicyContext context, + CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + if (!context.PublishedImages.TryGetValue(context.Target, out var publishedImage)) + { + return Task.FromResult(BuildPolicyResult.None); + } + + string[] expectedPlatformTags = context.Target.Platform.Tags + .Select(tag => tag.Name) + .ToArray(); + string[] expectedSharedTags = context.Target.Image.SharedTags + .Select(tag => tag.Name) + .ToArray(); + IReadOnlyList publishedPlatformTags = + publishedImage.Source == context.Target + ? publishedImage.Image.SimpleTags + : []; + IReadOnlyList publishedSharedTags = + publishedImage.Source == context.Target + ? publishedImage.SharedTags + : []; + bool tagsChanged = + !expectedPlatformTags.AreEquivalent(publishedPlatformTags) || + !expectedSharedTags.AreEquivalent(publishedSharedTags); + + BuildPolicyResult result = tagsChanged + ? new( + BuildAction.PublishExistingImage, + new BuildReason( + $"Platform tags changed from [{string.Join(", ", publishedPlatformTags)}] " + + $"to [{string.Join(", ", expectedPlatformTags)}]; shared tags changed from " + + $"[{string.Join(", ", publishedSharedTags)}] to " + + $"[{string.Join(", ", expectedSharedTags)}].")) + : BuildPolicyResult.None; + return Task.FromResult(result); + } +} + +public sealed class DockerfileChangedPolicy( + IGitService gitService, + string sourceRepoUrl) : IBuildPolicy +{ + private readonly IGitService _gitService = + gitService ?? throw new ArgumentNullException(nameof(gitService)); + + public Task EvaluateAsync( + BuildPolicyContext context, + CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + if (!context.PublishedImages.TryGetValue(context.Target, out var publishedImage)) + { + return Task.FromResult(BuildPolicyResult.None); + } + + string currentCommitUrl = _gitService.GetDockerfileCommitUrl( + context.Target.Platform, + sourceRepoUrl); + bool matches = publishedImage.Image.CommitUrl.Equals( + currentCommitUrl, + StringComparison.OrdinalIgnoreCase); + BuildPolicyResult result = matches + ? new( + BuildAction.NoAction, + new BuildReason($"Dockerfile is unchanged at '{currentCommitUrl}'.")) + : new( + BuildAction.BuildImage, + new BuildReason( + $"Dockerfile changed from '{publishedImage.Image.CommitUrl}' " + + $"to '{currentCommitUrl}'.")); + return Task.FromResult(result); + } +} + +public sealed class BaseImageChangedPolicy : IBuildPolicy +{ + private readonly ImageDigestCache _imageDigests; + private readonly ImageNameResolver _imageNames; + private readonly bool _isDryRun; + private readonly bool _useLocalExternalImage; + + private BaseImageChangedPolicy( + ImageDigestCache imageDigests, + ImageNameResolver imageNames, + bool useLocalExternalImage, + bool isDryRun) + { + _imageDigests = imageDigests ?? throw new ArgumentNullException(nameof(imageDigests)); + _imageNames = imageNames ?? throw new ArgumentNullException(nameof(imageNames)); + _useLocalExternalImage = useLocalExternalImage; + _isDryRun = isDryRun; + } + + public static BaseImageChangedPolicy FromLocalImages( + ImageDigestCache imageDigests, + ImageNameResolver imageNames, + bool isDryRun) => + new(imageDigests, imageNames, useLocalExternalImage: true, isDryRun); + + public static BaseImageChangedPolicy FromRegistry( + ImageDigestCache imageDigests, + ImageNameResolver imageNames, + bool isDryRun) => + new(imageDigests, imageNames, useLocalExternalImage: false, isDryRun); + + public async Task EvaluateAsync( + BuildPolicyContext context, + CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + if (!context.PublishedImages.TryGetValue(context.Target, out var publishedImage)) + { + return BuildPolicyResult.None; + } + + string? fromImage = context.Target.Platform.FinalStageFromImage; + if (fromImage is null) + { + return new( + BuildAction.NoAction, + new BuildReason("The final stage has no base image.")); + } + + string? currentDigest = context.Target.Platform.IsInternalFromImage(fromImage) + ? GetInternalBaseImageDigest(context, fromImage) + : await GetExternalBaseImageDigestAsync(context.Target, fromImage); + string publicImage = _imageNames.GetFromImagePublicTag(fromImage); + string? previousValue = GetDigestSha(publishedImage.Image.BaseImageDigest); + string? currentValue = GetDigestSha(currentDigest); + bool matches = previousValue?.Equals( + currentValue, + StringComparison.OrdinalIgnoreCase) == true; + + return matches + ? new( + BuildAction.NoAction, + new BuildReason( + $"Base image '{publicImage}' is unchanged at '{currentValue}'.")) + : new( + BuildAction.BuildImage, + new BuildReason( + $"Base image '{publicImage}' changed from " + + $"'{Display(previousValue)}' to '{Display(currentValue)}'.")); + } + + private static string? GetInternalBaseImageDigest( + BuildPolicyContext context, + string fromImage) + { + BuildTarget? parent = context.Graph.Parents[context.Target].FirstOrDefault(candidate => + candidate.Platform.Tags + .Concat(candidate.Image.SharedTags) + .Any(tag => tag.FullyQualifiedName == fromImage)); + return parent is not null && + context.PublishedImages.TryGetValue(parent, out var publishedImage) + ? publishedImage.Image.Digest + : null; + } + + private async Task GetExternalBaseImageDigestAsync( + BuildTarget target, + string fromImage) + { + if (_useLocalExternalImage) + { + string localImage = _imageNames.GetFromImageLocalTag(fromImage); + return await _imageDigests.GetLocalImageDigestAsync(localImage, _isDryRun); + } + + string registryImage = _imageNames.GetFinalStageImageNameForDigestQuery(target.Platform); + try + { + return await _imageDigests.GetManifestDigestShaAsync(registryImage, _isDryRun); + } + catch (Exception ex) when (IsImageNotFoundException(ex)) + { + return null; + } + } + + private static string? GetDigestSha(string? digest) => + string.IsNullOrWhiteSpace(digest) + ? null + : DockerHelper.GetDigestSha(digest); + + private static string Display(string? value) => value ?? ""; + + private static bool IsImageNotFoundException(Exception ex) => + ex is HttpRequestException { StatusCode: HttpStatusCode.NotFound } || + ex is RequestFailedException { Status: 404 }; +} diff --git a/src/ImageBuilder/Build/ImageDigestCache.cs b/src/ImageBuilder/Build/ImageDigestCache.cs new file mode 100644 index 000000000..858ecc504 --- /dev/null +++ b/src/ImageBuilder/Build/ImageDigestCache.cs @@ -0,0 +1,47 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.DotNet.ImageBuilder.Build; + +/// +/// Memoizes image digest lookups made while calculating and executing a build plan. +/// +public sealed class ImageDigestCache(Lazy manifestService) +{ + private readonly Lazy _inner = manifestService; + private readonly Dictionary _localDigestCache = []; + private readonly Dictionary _manifestDigestCache = []; + private readonly SemaphoreSlim _localDigestCacheLock = new(1); + private readonly SemaphoreSlim _manifestDigestCacheLock = new(1); + + public void AddDigest(string tag, string digest) + { + _localDigestCacheLock.Wait(); + try + { + _localDigestCache[tag] = digest; + } + finally + { + _localDigestCacheLock.Release(); + } + } + + public Task GetLocalImageDigestAsync(string tag, bool isDryRun) => + LockHelper.DoubleCheckedLockLookupAsync(_localDigestCacheLock, _localDigestCache, tag, + () => _inner.Value.GetLocalImageDigestAsync(tag, isDryRun), + // Don't allow null digests to be cached. A locally built image won't have a digest until + // it is pushed so if its digest is retrieved before pushing, we don't want that + // null to be cached. + val => !string.IsNullOrEmpty(val)); + + public Task GetManifestDigestShaAsync(string tag, bool isDryRun) => + LockHelper.DoubleCheckedLockLookupAsync(_manifestDigestCacheLock, _manifestDigestCache, tag, + () => _inner.Value.GetManifestDigestShaAsync(tag, isDryRun)); +} diff --git a/src/ImageBuilder/Commands/BuildCommand.cs b/src/ImageBuilder/Commands/BuildCommand.cs index 5ae85c30c..cd6070ffc 100644 --- a/src/ImageBuilder/Commands/BuildCommand.cs +++ b/src/ImageBuilder/Commands/BuildCommand.cs @@ -10,6 +10,7 @@ using System.Threading; using System.Threading.Tasks; using Azure.Core; +using Microsoft.DotNet.ImageBuilder.Build; using Microsoft.DotNet.ImageBuilder.Models.Image; using Microsoft.DotNet.ImageBuilder.ViewModel; @@ -25,7 +26,7 @@ public class BuildCommand : ManifestCommand private readonly Lazy _manifestService; private readonly IRegistryCredentialsProvider _registryCredentialsProvider; private readonly IAzureTokenCredentialProvider _tokenCredentialProvider; - private readonly IImageCacheService _imageCacheService; + private readonly BuildPlanner _buildPlanner; private readonly ImageDigestCache _imageDigestCache; private readonly List _processedTags = new List(); private readonly HashSet _builtPlatforms = new(); @@ -38,7 +39,9 @@ public class BuildCommand : ManifestCommand /// private readonly Dictionary _sourceDigestCopyLocationMapping = new(); + private BuildGraph? _buildGraph; private ImageArtifactDetails? _imageArtifactDetails; + private bool _hasPublishedImagesToUse; public BuildCommand( IManifestJsonService manifestJsonService, @@ -50,7 +53,7 @@ public BuildCommand( IManifestServiceFactory manifestServiceFactory, IRegistryCredentialsProvider registryCredentialsProvider, IAzureTokenCredentialProvider tokenCredentialProvider, - IImageCacheService imageCacheService) : base(manifestJsonService) + BuildPlanner buildPlanner) : base(manifestJsonService) { _dockerService = new DockerServiceCache(dockerService ?? throw new ArgumentNullException(nameof(dockerService))); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); @@ -59,7 +62,7 @@ public BuildCommand( _copyImageService = copyImageService ?? throw new ArgumentNullException(nameof(copyImageService)); _registryCredentialsProvider = registryCredentialsProvider ?? throw new ArgumentNullException(nameof(registryCredentialsProvider)); _tokenCredentialProvider = tokenCredentialProvider ?? throw new ArgumentNullException(nameof(tokenCredentialProvider)); - _imageCacheService = imageCacheService ?? throw new ArgumentNullException(nameof(imageCacheService)); + _buildPlanner = buildPlanner ?? throw new ArgumentNullException(nameof(buildPlanner)); // Lazily create services which need access to options ArgumentNullException.ThrowIfNull(manifestServiceFactory); @@ -100,10 +103,20 @@ public override async Task ExecuteAsync() _imageArtifactDetails = new ImageArtifactDetails(); } - await ExecuteWithDockerCredentialsAsync(PullBaseImagesAsync); - await BuildImagesAsync(); + ImageArtifactDetails? publishedImages = Options.ImageInfoSourcePath is null + ? null + : ImageInfoHelper.LoadFromFile( + Options.ImageInfoSourcePath, + Manifest, + skipManifestValidation: true); + _buildGraph = BuildGraph.CreateFiltered(Manifest); + await ExecuteWithDockerCredentialsAsync(() => PullBaseImagesAsync(_buildGraph)); + IReadOnlyList plan = await CreateBuildPlanAsync( + _buildGraph, + publishedImages); + await BuildImagesAsync(plan); - if (_processedTags.Count > 0 || _imageCacheService.HasAnyCachedPlatforms) + if (_processedTags.Count > 0 || _hasPublishedImagesToUse) { // Log in again to refresh token as it may have expired from a long build await ExecuteWithDockerCredentialsAsync(async () => @@ -161,19 +174,10 @@ private async Task PublishImageInfoAsync() throw new InvalidOperationException("Source repo URL must be provided when outputting to an image info file."); } - Dictionary platformDataByTag = new Dictionary(); - foreach (PlatformData platformData in GetProcessedPlatforms()) - { - if (platformData.PlatformInfo is not null) - { - foreach (TagInfo tag in platformData.PlatformInfo.Tags) - { - platformDataByTag.Add(tag.FullyQualifiedName, platformData); - } - } - } - IEnumerable processedPlatforms = GetProcessedPlatforms(); + Dictionary platformDataByPlatform = processedPlatforms + .Where(platform => platform.PlatformInfo is not null) + .ToDictionary(platform => platform.PlatformInfo!); List platformsWithNoPushTags = new List(); foreach (PlatformData platform in processedPlatforms) @@ -185,7 +189,7 @@ private async Task PublishImageInfoAsync() if (Options.IsPushEnabled) { await SetPlatformDataDigestAsync(platform, tag.FullyQualifiedName); - SetPlatformDataBaseDigest(platform, platformDataByTag); + SetPlatformDataBaseDigest(platform, platformDataByPlatform); await SetPlatformDataLayersAsync(platform, tag.FullyQualifiedName); } @@ -235,12 +239,26 @@ private void SetPlatformDataCreatedDate(PlatformData platform, string tag) platform.Created = createdDate; } - private void SetPlatformDataBaseDigest(PlatformData platform, Dictionary platformDataByTag) + private void SetPlatformDataBaseDigest( + PlatformData platform, + IReadOnlyDictionary platformDataByPlatform) { string? baseImageDigest = platform.BaseImageDigest; if (platform.BaseImageDigest is null && platform.PlatformInfo?.FinalStageFromImage is not null) { - if (!platformDataByTag.TryGetValue(platform.PlatformInfo.FinalStageFromImage, out PlatformData? basePlatformData)) + BuildGraph graph = _buildGraph ?? + throw new InvalidOperationException("Build graph has not been created."); + BuildTarget target = graph.Targets.First( + target => target.Platform == platform.PlatformInfo); + BuildTarget? parent = graph.Parents[target].SingleOrDefault(parent => + parent.Platform.Tags + .Concat(parent.Image.SharedTags) + .Any(tag => + tag.FullyQualifiedName == platform.PlatformInfo.FinalStageFromImage)); + if (parent is null || + !platformDataByPlatform.TryGetValue( + parent.Platform, + out PlatformData? basePlatformData)) { throw new InvalidOperationException( $"Unable to find platform data for tag '{platform.PlatformInfo.FinalStageFromImage}'. " + @@ -300,30 +318,57 @@ private async Task SetPlatformDataDigestAsync(PlatformData platform, string tag) platform.Digest = digest; } - private async Task BuildImagesAsync() + private Task> CreateBuildPlanAsync( + BuildGraph graph, + ImageArtifactDetails? publishedImages) + { + IBuildPolicy policy = Options.NoCache + ? new AlwaysBuildPolicy() + : CompositeBuildPolicy.ImageCache( + BuildAction.UsePublishedImage, + BaseImageChangedPolicy.FromLocalImages( + _imageDigestCache, + _imageNameResolver.Value, + Options.IsDryRun), + new DockerfileChangedPolicy( + _gitService, + Options.SourceRepoUrl ?? string.Empty)); + + return _buildPlanner.CreatePlanAsync( + graph, + publishedImages, + policy); + } + + private async Task BuildImagesAsync(IReadOnlyList plan) { _logger.LogInformation("BUILDING IMAGES"); - ImageArtifactDetails? srcImageArtifactDetails = null; - if (Options.ImageInfoSourcePath != null) - { - srcImageArtifactDetails = ImageInfoHelper.LoadFromFile(Options.ImageInfoSourcePath, Manifest, skipManifestValidation: true); - } + BuildPlanItem[] executableItems = plan + .Where(item => item.Action != BuildAction.NoAction) + .ToArray(); + _hasPublishedImagesToUse = executableItems.Any( + item => item.Action is + BuildAction.UsePublishedImage or + BuildAction.PublishExistingImage); - foreach (RepoInfo repoInfo in Manifest.FilteredRepos) + foreach (IGrouping repoPlan in executableItems + .GroupBy(item => item.Target.Repo)) { + RepoInfo repoInfo = repoPlan.Key; RepoData repoData = CreateRepoData(repoInfo); - RepoData? srcRepoData = srcImageArtifactDetails?.Repos.FirstOrDefault(srcRepo => srcRepo.Repo == repoInfo.Name); - foreach (ImageInfo image in repoInfo.FilteredImages) + foreach (IGrouping imagePlan in repoPlan + .GroupBy(item => item.Target.Image)) { + ImageInfo image = imagePlan.Key; ImageData imageData = CreateImageData(image); repoData.Images.Add(imageData); - ImageData? srcImageData = srcRepoData?.Images.FirstOrDefault(srcImage => srcImage.ManifestImage == image); - - foreach (PlatformInfo platform in image.FilteredPlatforms) + foreach (BuildPlanItem plannedImage in imagePlan) { + PlatformInfo platform = plannedImage.Target.Platform; + // Tag the built images with the shared tags as well as the platform tags. // Some tests and image FROM instructions depend on these tags. @@ -338,34 +383,27 @@ private async Task BuildImagesAsync() PlatformData platformData = CreatePlatformData(image, platform); imageData.Platforms.Add(platformData); - bool isCachedImage = false; - if (!Options.NoCache) + if (plannedImage.Action is + BuildAction.UsePublishedImage or + BuildAction.PublishExistingImage) { - ImageCacheResult cacheResult = await _imageCacheService.CheckForCachedImageAsync( - srcImageData, - platformData, - _imageDigestCache, - _imageNameResolver.Value, - sourceRepoUrl: Options.SourceRepoUrl, - isLocalBaseImageExpected: true, - isDryRun: Options.IsDryRun); - - if (cacheResult.State.HasFlag(ImageCacheState.Cached)) - { - isCachedImage = true; - - CopyPlatformDataFromCachedPlatform(platformData, cacheResult.Platform!); - platformData.IsUnchanged = cacheResult.State != ImageCacheState.CachedWithMissingTags; - - await OnCacheHitAsync(repoInfo, allTagInfos, pullImage: cacheResult.IsNewCacheHit, cacheResult.Platform!.Digest); - } + PublishedImage publishedImage = plannedImage.PublishedImage ?? + throw new InvalidOperationException( + $"Build plan did not provide reusable metadata for '{platform.DockerfilePath}'."); + + CopyPlatformDataFromCachedPlatform(platformData, publishedImage.Image); + platformData.IsUnchanged = + plannedImage.Action == BuildAction.UsePublishedImage; + await UsePublishedImageAsync( + repoInfo, + allTagInfos, + publishedImage.Image.Digest); } - - if (!isCachedImage) + else if (plannedImage.Action == BuildAction.BuildImage) { _processedTags.AddRange(allTagInfos); - BuildImage(platform, allTags); + BuildImage(plannedImage.Target, allTags); _builtPlatforms.Add(platformData); if (Options.IsPushEnabled && platform.FinalStageFromImage is not null) @@ -385,12 +423,14 @@ await _imageDigestCache.GetLocalImageDigestAsync( } } - private void CopyPlatformDataFromCachedPlatform(PlatformData dstPlatform, PlatformData srcPlatform) + private void CopyPlatformDataFromCachedPlatform( + PlatformData destination, + PlatformData source) { // When a cache hit occurs for a Dockerfile, we want to transfer some of the metadata about the previously // published image so we don't need to recalculate it again. - dstPlatform.BaseImageDigest = srcPlatform.BaseImageDigest; - dstPlatform.Layers = new List(srcPlatform.Layers); + destination.BaseImageDigest = source.BaseImageDigest; + destination.Layers = new List(source.Layers); } private RepoData CreateRepoData(RepoInfo repoInfo) => @@ -479,11 +519,16 @@ private void ValidatePlatformIsCompatibleWithBaseImage(PlatformInfo platform) } } - private void BuildImage(PlatformInfo platform, IEnumerable allTags) + private void BuildImage( + BuildTarget target, + IEnumerable allTags) { + PlatformInfo platform = target.Platform; ValidatePlatformIsCompatibleWithBaseImage(platform); - bool createdPrivateDockerfile = UpdateDockerfileFromCommands(platform, out string dockerfilePath); + bool createdPrivateDockerfile = UpdateDockerfileFromCommands( + target, + out string dockerfilePath); try { @@ -558,10 +603,13 @@ private void BuildImage(PlatformInfo platform, IEnumerable allTags) private IEnumerable GetDockerBuildOptions() => Options.DockerBuildOptions.Where(option => !string.IsNullOrWhiteSpace(option)); - private async Task OnCacheHitAsync(RepoInfo repo, IEnumerable allTags, bool pullImage, string sourceDigest) + private async Task UsePublishedImageAsync( + RepoInfo repo, + IEnumerable allTags, + string sourceDigest) { _logger.LogInformation(string.Empty); - _logger.LogInformation("CACHE HIT"); + _logger.LogInformation("USING PUBLISHED IMAGE"); _logger.LogInformation(string.Empty); // When a cache hit occurs on an image, we copy the image from its source location (e.g. mcr.microsoft.com) to its @@ -571,13 +619,13 @@ private async Task OnCacheHitAsync(RepoInfo repo, IEnumerable allTags, // The pulled image is then tagged with the same tags it would be tagged with had it been built locally. This allows // dependent Dockerfiles that reference those tags to seamlessly consume the pulled image. + bool pullImage = !_sourceDigestCopyLocationMapping.ContainsKey(sourceDigest); string copiedSourceDigest = sourceDigest; if (Options.IsPushEnabled) { copiedSourceDigest = await CopyCachedImage(allTags, sourceDigest); } - // Pull the image instead of building it if (pullImage) { await ExecuteWithDockerCredentialsAsync(() => @@ -635,7 +683,7 @@ await _copyImageService.ImportImageAsync( return sourceDigest; } - private async Task PullBaseImagesAsync() + private async Task PullBaseImagesAsync(BuildGraph graph) { _logger.LogInformation("PULLING LATEST BASE IMAGES"); @@ -647,7 +695,10 @@ private async Task PullBaseImagesAsync() HashSet pulledTags = []; HashSet externalFromImages = []; - foreach (PlatformInfo platform in Manifest.GetFilteredPlatforms()) + PlatformInfo[] platforms = graph.Targets + .Select(target => target.Platform) + .ToArray(); + foreach (PlatformInfo platform in platforms) { IEnumerable platformExternalFromImages = platform.ExternalFromImages.Distinct(); externalFromImages.UnionWith(platformExternalFromImages); @@ -672,7 +723,7 @@ private async Task PullBaseImagesAsync() } IEnumerable finalStageExternalFromImages = - Manifest.GetFilteredPlatforms() + platforms .Where(platform => platform.FinalStageFromImage is not null && !platform.IsInternalFromImage(platform.FinalStageFromImage)) @@ -727,8 +778,11 @@ private void PushImages() } } - private bool UpdateDockerfileFromCommands(PlatformInfo platform, out string dockerfilePath) + private bool UpdateDockerfileFromCommands( + BuildTarget target, + out string dockerfilePath) { + PlatformInfo platform = target.Platform; bool updateDockerfile = false; dockerfilePath = platform.DockerfilePath; @@ -739,9 +793,7 @@ private bool UpdateDockerfileFromCommands(PlatformInfo platform, out string dock foreach (string fromImage in platform.OverriddenFromImages) { - string fromRepo = DockerHelper.GetRepo(fromImage); - RepoInfo repo = Manifest.FilteredRepos.First(r => r.FullModelName == fromRepo); - string newFromImage = DockerHelper.ReplaceRepo(fromImage, repo.QualifiedName); + string newFromImage = target.FromImageOverrides[fromImage]; _logger.LogInformation($"Replacing FROM `{fromImage}` with `{newFromImage}`"); Regex fromRegex = new Regex($@"FROM\s+{Regex.Escape(fromImage)}[^\s\r\n]*"); dockerfileContents = fromRegex.Replace(dockerfileContents, $"FROM {newFromImage}"); diff --git a/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs b/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs index 53918306d..eca05ef91 100644 --- a/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs +++ b/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs @@ -3,11 +3,11 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Text.RegularExpressions; using System.Threading.Tasks; +using Microsoft.DotNet.ImageBuilder.Build; using Microsoft.DotNet.ImageBuilder.Models.Image; using Microsoft.DotNet.ImageBuilder.Models.Manifest; using Microsoft.DotNet.ImageBuilder.ViewModel; @@ -21,14 +21,22 @@ public class GenerateBuildMatrixCommand : ManifestCommand _imageArtifactDetails; private static readonly char[] s_pathSeparators = { '/', '\\' }; private static readonly Regex s_versionRegex = new(@$"^(?<{VersionRegGroupName}>(\d|\.)+).*$"); - private readonly IImageCacheService _imageCacheService; + private readonly BuildPlanner _buildPlanner; + private readonly IGitService _gitService; private readonly ILogger _logger; private readonly ImageDigestCache _imageDigestCache; private readonly Lazy _imageNameResolver; - - public GenerateBuildMatrixCommand(IManifestJsonService manifestJsonService, IImageCacheService imageCacheService, IManifestServiceFactory manifestServiceFactory, ILogger logger) : base(manifestJsonService) + private BuildGraph? _dependencyGraph; + + public GenerateBuildMatrixCommand( + IManifestJsonService manifestJsonService, + BuildPlanner buildPlanner, + IGitService gitService, + IManifestServiceFactory manifestServiceFactory, + ILogger logger) : base(manifestJsonService) { - _imageCacheService = imageCacheService ?? throw new ArgumentNullException(nameof(imageCacheService)); + _buildPlanner = buildPlanner ?? throw new ArgumentNullException(nameof(buildPlanner)); + _gitService = gitService ?? throw new ArgumentNullException(nameof(gitService)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _imageArtifactDetails = new Lazy(() => { @@ -104,7 +112,7 @@ private void AddDockerfilePathLegs( { // Pass 1: Find direct dependencies from the Dockerfile's FROM statement IEnumerable> subgraphs = platformGrouping.GetCompleteSubgraphs( - platform => Manifest.GetParents(platform, platformGrouping)); + platform => GetParents(platform, platformGrouping)); // Pass 2: Combine subgraphs that have a common Dockerfile path for the root image subgraphs = ConsolidateSubgraphs(subgraphs, platform => platform.DockerfilePath); @@ -200,7 +208,9 @@ private IEnumerable GetCustomLegGroupPlatforms(PlatformInfo platfo .Select(dependency => Manifest.GetPlatformByTag(dependency)); return dependencyPlatforms .Concat(dependencyPlatforms - .SelectMany(dependencyPlatform => Manifest.GetAncestors(dependencyPlatform, Manifest.GetFilteredPlatforms()))); + .SelectMany(dependencyPlatform => GetAncestors( + dependencyPlatform, + Manifest.GetFilteredPlatforms()))); }) .Distinct(); } @@ -279,7 +289,7 @@ private void AddVersionedOsLegs(BuildMatrixInfo matrix, // of platforms in Pass 2 to only test what was already built. IEnumerable> subgraphs = platformGrouping .GetCompleteSubgraphs(platform => - Manifest.GetParents(platform, allPlatforms) + GetParents(platform, allPlatforms) .Union(GetCustomLegGroupPlatforms(platform, CustomBuildLegDependencyType.Integral))); // Pass 2: Filter subgraphs to only images that are in the current platform group @@ -297,7 +307,7 @@ private void AddVersionedOsLegs(BuildMatrixInfo matrix, // Pass 5: Append the parent graph of each platform to each respective subgraph subgraphs = subgraphs.GetCompleteSubgraphs( - subgraph => subgraph.Select(platform => Manifest.GetAncestors(platform, platformGrouping))) + subgraph => subgraph.Select(platform => GetAncestors(platform, platformGrouping))) .Select(set => set .SelectMany(subgraph => subgraph) .Distinct()) @@ -409,95 +419,82 @@ private static string FormatMatrixName(IEnumerable parts) private async Task> GetPlatformsAsync() { - IEnumerable filteredRepos = Manifest.FilteredRepos.ToList(); + _dependencyGraph = BuildGraph.Create(Manifest); + ImageArtifactDetails? imageInfo = _imageArtifactDetails.Value; + HashSet completedPlatforms = imageInfo?.Repos + .SelectMany(repo => repo.Images) + .SelectMany(image => image.Platforms) + .Where(platform => platform.IsUnchanged && platform.PlatformInfo is not null) + .Select(platform => platform.PlatformInfo!) + .ToHashSet() + ?? []; + BuildGraph graph = BuildGraph.CreateFiltered( + Manifest, + platform => !completedPlatforms.Contains(platform)); + IBuildPolicy policy = Options.TrimCachedImages + ? CompositeBuildPolicy.ImageCache( + BuildAction.NoAction, + BaseImageChangedPolicy.FromRegistry( + _imageDigestCache, + _imageNameResolver.Value, + Options.IsDryRun), + new DockerfileChangedPolicy( + _gitService, + Options.SourceRepoUrl ?? string.Empty)) + : new AlwaysBuildPolicy("The image was selected for a build."); + IReadOnlyList plan = await _buildPlanner.CreatePlanAsync( + graph, + imageInfo, + policy); + + IEnumerable plannedPlatforms = plan + .Where(item => item.Action != BuildAction.NoAction) + .Select(item => item.Target.Platform); + return Options.TrimCachedImages + ? plannedPlatforms.OrderBy(platform => platform.DockerfilePath) + : plannedPlatforms; + } - if (_imageArtifactDetails.Value is null) + private IEnumerable GetParents( + PlatformInfo platform, + IEnumerable availablePlatforms) + { + if (_dependencyGraph is null) { - return filteredRepos.SelectMany(repo => repo.FilteredImages).SelectMany(image => image.FilteredPlatforms); + throw new InvalidOperationException( + "The dependency graph must be created before generating matrix legs."); } - IEnumerable<(PlatformInfo PlatformInfo, ImageData? ImageData, PlatformData? PlatformData)> platformMappings = - filteredRepos.SelectMany(repo => - repo.FilteredImages - .SelectMany(image => image.FilteredPlatforms) - .Select(platform => - { - (PlatformData Platform, ImageData Image)? matchingPlatform = ImageInfoHelper.GetMatchingPlatformData(platform, repo, _imageArtifactDetails.Value); - return (platform, matchingPlatform?.Image, matchingPlatform?.Platform); - }) - .Where(platformMapping => platformMapping.Platform is null || !platformMapping.Platform.IsUnchanged)); + HashSet available = availablePlatforms.ToHashSet(); + BuildTarget target = _dependencyGraph.Targets.First( + target => target.Platform == platform); + return _dependencyGraph.Parents[target] + .Select(parent => parent.Platform) + .Where(available.Contains); + } - if (!Options.TrimCachedImages) - { - return platformMappings.Select(platformMapping => platformMapping.PlatformInfo); - } + private IEnumerable GetAncestors( + PlatformInfo platform, + IEnumerable availablePlatforms) + { + HashSet available = availablePlatforms.ToHashSet(); + HashSet ancestors = []; + Queue remaining = new(GetParents(platform, available)); - _logger.LogInformation("Trimming platforms based on image cache state..."); - - // Here we will trim the platforms based on their image cache state. This reduces the amount of jobs that need to - // be run. Otherwise, you may spin up a bunch of jobs that end up processing a bunch of cached images and - // essentially becomes a no-op. - - // We need to group the platforms according to their parent dependency hierarchy. This is important because we must - // treat the hierarchy as a unit. For example, if runtime-deps is cached but runtime (which is a descendant of - // runtime-deps) is not, then we need to ensure that both runtime-deps and runtime is included. We do not want to - // trim just the runtime-deps platform in that case. - IEnumerable> subgraphs = - platformMappings.GetCompleteSubgraphs( - platformGrouping => - Manifest.GetParents( - platformGrouping.PlatformInfo, - platformMappings.Select(m => m.PlatformInfo) - ).Select(platformInfo => platformMappings.First(mapping => mapping.PlatformInfo == platformInfo))); - - ConcurrentBag nonCachedPlatforms = []; - await Parallel.ForEachAsync(subgraphs, async (subgraph, _) => + while (remaining.TryDequeue(out PlatformInfo? ancestor)) { - ConcurrentBag subgraphNonCachedPlatforms = []; - await Parallel.ForEachAsync(subgraph, async (platformMapping, _) => + if (!ancestors.Add(ancestor)) { - if (platformMapping.PlatformData is null) - { - _logger.LogInformation($"Image info not found for '{platformMapping.PlatformInfo.DockerfilePath}'. Including path in matrix."); - subgraphNonCachedPlatforms.Add(platformMapping.PlatformInfo); - return; - } - - ImageCacheResult cacheResult = await _imageCacheService.CheckForCachedImageAsync( - platformMapping.ImageData, - platformMapping.PlatformData, - _imageDigestCache, - _imageNameResolver.Value, - Options.SourceRepoUrl, - isLocalBaseImageExpected: false, - Options.IsDryRun); - - bool includePlatformInMatrix = !cacheResult.State.HasFlag(ImageCacheState.Cached); - - _logger.LogInformation( - $"Image '{platformMapping.PlatformInfo.DockerfilePath}' cache state is {cacheResult.State}. Included in matrix: {includePlatformInMatrix}"); + continue; + } - if (includePlatformInMatrix) - { - subgraphNonCachedPlatforms.Add(platformMapping.PlatformInfo); - } - }); - - // As mentioned above, we need to treat the hierarchy as a unit so even though a subset of the platforms - // in the hierarchy may be cached, they all need to be included. Only in the case where they're all - // cached, should they be excluded. To determine what needs to be included, it can be simplified to just - // check whether there are any platforms identified within the hierarchy as not being cached. If so, then - // include the whole hierarchy as non-cached platforms. - if (!subgraphNonCachedPlatforms.IsEmpty) + foreach (PlatformInfo parent in GetParents(ancestor, available)) { - foreach ((PlatformInfo PlatformInfo, ImageData? ImageData, PlatformData? PlatformData) platformMapping in subgraph) - { - nonCachedPlatforms.Add(platformMapping.PlatformInfo); - } + remaining.Enqueue(parent); } - }); + } - return nonCachedPlatforms.OrderBy(platform => platform.DockerfilePath); + return ancestors; } public async Task> GenerateMatrixInfoAsync() diff --git a/src/ImageBuilder/Commands/GetStaleImagesCommand.cs b/src/ImageBuilder/Commands/GetStaleImagesCommand.cs index a21866026..1e68e256c 100644 --- a/src/ImageBuilder/Commands/GetStaleImagesCommand.cs +++ b/src/ImageBuilder/Commands/GetStaleImagesCommand.cs @@ -5,8 +5,8 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Threading; using System.Threading.Tasks; +using Microsoft.DotNet.ImageBuilder.Build; using Microsoft.DotNet.ImageBuilder.Models.Image; using Microsoft.DotNet.ImageBuilder.ViewModel; using Newtonsoft.Json; @@ -16,8 +16,8 @@ namespace Microsoft.DotNet.ImageBuilder.Commands { public class GetStaleImagesCommand : Command { - private readonly Dictionary _imageDigests = new(); - private readonly SemaphoreSlim _imageDigestsLock = new(1); + private readonly BuildPlanner _buildPlanner; + private readonly ImageDigestCache _imageDigestCache; private readonly Lazy _manifestService; private readonly IManifestJsonService _manifestJsonService; private readonly ILogger _logger; @@ -29,12 +29,14 @@ public GetStaleImagesCommand( IManifestJsonService manifestJsonService, ILogger logger, IOctokitClientFactory octokitClientFactory, - IGitService gitService) + IGitService gitService, + BuildPlanner buildPlanner) { _manifestJsonService = manifestJsonService ?? throw new ArgumentNullException(nameof(manifestJsonService)); _logger = logger; _octokitClientFactory = octokitClientFactory; _gitService = gitService; + _buildPlanner = buildPlanner ?? throw new ArgumentNullException(nameof(buildPlanner)); // Don't worry about authenticating to our own ACR, since we are checking base image digests from public // registries instead of our staging location. Registry credentials are needed however to prevent rate @@ -42,6 +44,7 @@ public GetStaleImagesCommand( ArgumentNullException.ThrowIfNull(manifestServiceFactory); _manifestService = new Lazy(() => manifestServiceFactory.Create(Options.CredentialsOptions)); + _imageDigestCache = new ImageDigestCache(_manifestService); } protected override string Description => "Gets paths to images whose base images are out-of-date"; @@ -97,110 +100,25 @@ private async Task> GetPathsToRebuildAsync(Models.Subscripti repoPrefix: null, sourceRepoPrefix: Options.SourceRepoPrefix); - List pathsToRebuild = new(); - - foreach (RepoInfo repo in manifest.FilteredRepos) - { - IEnumerable platforms = repo.FilteredImages - .SelectMany(image => image.FilteredPlatforms) - .Where(platform => platform.FinalStageFromImage is not null && !platform.IsInternalFromImage(platform.FinalStageFromImage)); - - foreach (PlatformInfo platform in platforms) - { - pathsToRebuild.AddRange( - await GetPathsToRebuildAsync(manifest, platform, repo, imageArtifactDetails, imageNameResolver)); - } - } - - return pathsToRebuild.Distinct().ToList(); - } - - private static IEnumerable GetDescendants(PlatformInfo platform, ManifestInfo manifest) => - manifest.GetDescendants(platform, manifest.GetAllPlatforms().ToList(), includeAncestorsOfDescendants: true) - .Prepend(platform); - - private async Task> GetPathsToRebuildAsync( - ManifestInfo manifest, - PlatformInfo platform, - RepoInfo repo, - ImageArtifactDetails imageArtifactDetails, - ImageNameResolverForMatrix imageNameResolver) - { - string? fromImage = platform.FinalStageFromImage; - if (fromImage is null) - { - _logger.LogInformation( - "Dockerfile {DockerfilePath} has no base image. It is automatically considered up-to-date.", - platform.DockerfilePath); - - return []; - } - - (PlatformData Platform, ImageData Image)? matchingPlatform = - ImageInfoHelper.GetMatchingPlatformData(platform, repo, imageArtifactDetails); - - if (matchingPlatform is null) - { - _logger.LogWarning( - "Image info not found for '{DockerfilePath}'. It will be queued for rebuild.", - platform.DockerfilePath); - - IEnumerable dependentPlatforms = GetDescendants(platform, manifest); - return dependentPlatforms.Select(p => p.Model.Dockerfile).ToList(); - } - - // Resolve where to actually fetch the digest from. For external base images this - // points to the mirror location in the staging registry; for internal images it is the - // original FROM tag. The "public" form is the canonical reference matching what gets - // recorded in image-info.json and so is the right repo to use in the digest comparison - // string below. - string baseImagePullReference = imageNameResolver.GetFromImagePullTag(fromImage); - string baseImagePublicReference = imageNameResolver.GetFromImagePublicTag(fromImage); - - // Cache the manifest digest by pull reference. The digest is a function of where we - // actually pull bytes from, so the pull reference is the correct cache key. - string baseImageManifestDigest = - await LockHelper.DoubleCheckedLockLookupAsync( - semaphore: _imageDigestsLock, - dictionary: _imageDigests, - key: baseImagePullReference, - getValue: () => - // This reaches out to the registry to fetch the digest from the pull - // reference. For external images, this fetches from the mirror. - _manifestService.Value.GetManifestDigestShaAsync(baseImagePullReference, Options.IsDryRun)); - - // Build a digest-pinned reference of the form '@sha256:' (e.g. - // 'mcr.microsoft.com/dotnet/runtime@sha256:abc123...'). This must be built per-call - // from this platform's own public reference — two FROM spellings (e.g. 'almalinux:8' - // vs 'library/almalinux:8') can share a pull reference but resolve to different - // public references, so the formed string cannot be cached or shared across platforms. - // The shape matches what's stored in Platform.BaseImageDigest so the equality check - // below is meaningful. - string currentBaseImageDigestReference = - DockerHelper.GetDigestString( - repo: DockerHelper.GetRepo(baseImagePublicReference), - sha: baseImageManifestDigest); - - bool shouldRebuildImage = matchingPlatform.Value.Platform.BaseImageDigest != currentBaseImageDigestReference; - - _logger.LogInformation( - "Dockerfile {DockerfilePath} was last built with base image {BaseImagePublicReference} at digest" - + " {LastBuildBaseImageDigestReference}. Image {BaseImagePullReference} has current digest" - + " {CurrentBaseImageDigestReference}. Up to date: {IsUpToDate}.", - platform.DockerfilePath, - baseImagePublicReference, - matchingPlatform.Value.Platform.BaseImageDigest, - baseImagePullReference, - currentBaseImageDigestReference, - !shouldRebuildImage); - - if (shouldRebuildImage) - { - IEnumerable dependentPlatforms = GetDescendants(platform, manifest); - return dependentPlatforms.Select(p => p.Model.Dockerfile).ToList(); - } - - return []; + BuildGraph graph = BuildGraph.CreateFiltered(manifest); + string sourceRepoUrl = + $"https://github.com/{subscription.Manifest.Owner}/{subscription.Manifest.Repo}"; + IReadOnlyList plan = await _buildPlanner.CreatePlanAsync( + graph, + imageArtifactDetails, + CompositeBuildPolicy.ImageCache( + BuildAction.NoAction, + BaseImageChangedPolicy.FromRegistry( + _imageDigestCache, + imageNameResolver, + Options.IsDryRun), + new DockerfileChangedPolicy(_gitService, sourceRepoUrl))); + + return plan + .Where(item => item.Action != BuildAction.NoAction) + .Select(item => item.Target.Platform.Model.Dockerfile) + .Distinct() + .ToArray(); } private async Task GetImageInfoForSubscriptionAsync(Models.Subscription.Subscription subscription, ManifestInfo manifest) diff --git a/src/ImageBuilder/ImageBuilder.cs b/src/ImageBuilder/ImageBuilder.cs index fe496a706..d4e749470 100644 --- a/src/ImageBuilder/ImageBuilder.cs +++ b/src/ImageBuilder/ImageBuilder.cs @@ -4,6 +4,7 @@ using System; using System.Threading; +using Microsoft.DotNet.ImageBuilder.Build; using Microsoft.DotNet.ImageBuilder.Commands; using Microsoft.DotNet.ImageBuilder.Commands.Signing; using Microsoft.DotNet.ImageBuilder.Configuration; @@ -96,7 +97,7 @@ public static IHost CreateAppHost() "image-builder-oras-timeout", pipeline => pipeline.AddTimeout(TimeSpan.FromSeconds(10))); - builder.Services.AddSingleton(); + builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddMemoryCache(); diff --git a/src/ImageBuilder/ImageCacheService.cs b/src/ImageBuilder/ImageCacheService.cs deleted file mode 100644 index 7641d99a8..000000000 --- a/src/ImageBuilder/ImageCacheService.cs +++ /dev/null @@ -1,304 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; -using System.Collections.Generic; -using System.Linq; -using System.Net; -using System.Net.Http; -using System.Threading.Tasks; -using Azure; -using Microsoft.DotNet.ImageBuilder.Models.Image; -using Microsoft.DotNet.ImageBuilder.ViewModel; - -namespace Microsoft.DotNet.ImageBuilder; - -/// -/// Service for determining whether a Docker image can be retrieved from a cache -/// rather than being rebuilt. -/// -public interface IImageCacheService -{ - /// - /// Gets whether any platforms have been identified as cached during this session. - /// - bool HasAnyCachedPlatforms { get; } - - /// - /// Checks whether a previously built image can be reused from cache. - /// - /// Image data from the source image-info file, if available. - /// Platform data for the image being checked. - /// Cache for looking up image digests from registries. - /// Resolver for constructing image names for digest queries. - /// URL of the source repository containing the Dockerfiles. - /// Whether the base image is expected to exist locally rather than in a remote registry. - /// Whether this is a dry run that should skip actual registry calls. - Task CheckForCachedImageAsync( - ImageData? srcImageData, - PlatformData platformData, - ImageDigestCache imageDigestCache, - ImageNameResolver imageNameResolver, - string? sourceRepoUrl, - bool isLocalBaseImageExpected, - bool isDryRun); -} - -/// -public class ImageCacheService : IImageCacheService -{ - private readonly ILogger _logger; - private readonly IGitService _gitService; - - private readonly object _cachedPlatformsLock = new(); - - /// - /// Metadata about Dockerfiles whose images have been retrieved from the cache. - /// Keyed by the build cache key derived from the platform's Dockerfile path and build args. - /// - private readonly Dictionary _cachedPlatforms = []; - - public ImageCacheService(ILogger logger, IGitService gitService) - { - _logger = logger ?? throw new ArgumentNullException(nameof(logger)); - _gitService = gitService ?? throw new ArgumentNullException(nameof(gitService)); - } - - /// - public bool HasAnyCachedPlatforms - { - get - { - lock (_cachedPlatformsLock) - { - return _cachedPlatforms.Any(); - } - } - } - - /// - public async Task CheckForCachedImageAsync( - ImageData? srcImageData, - PlatformData platformData, - ImageDigestCache imageDigestCache, - ImageNameResolver imageNameResolver, - string? sourceRepoUrl, - bool isLocalBaseImageExpected, - bool isDryRun) - { - ImageCacheState cacheState = ImageCacheState.NotCached; - bool isNewCacheHit = false; - PlatformData? srcPlatformData = srcImageData?.Platforms - .FirstOrDefault(srcPlatform => srcPlatform.PlatformInfo == platformData.PlatformInfo); - - if (platformData.PlatformInfo is null) - { - throw new Exception("Expected platform info to be set"); - } - - string cacheKey = GetBuildCacheKey(platformData.PlatformInfo); - lock (_cachedPlatformsLock) - { - if (_cachedPlatforms.TryGetValue(cacheKey, out PlatformData? cachedPlatform)) - { - cacheState = ImageCacheState.Cached; - if (srcPlatformData is null || - !CachedPlatformHasAllTagsPublished(srcPlatformData)) - { - cacheState = ImageCacheState.CachedWithMissingTags; - } - return new ImageCacheResult(cacheState, isNewCacheHit, cachedPlatform); - } - } - - // If this Dockerfile has been built and published before - if (srcPlatformData != null) - { - bool isCachedImage = await CheckForCachedImageFromImageInfoAsync( - platformData.PlatformInfo, - srcPlatformData, - imageDigestCache, - imageNameResolver, - sourceRepoUrl, - isLocalBaseImageExpected, - isDryRun); - - if (isCachedImage) - { - isNewCacheHit = true; - cacheState = ImageCacheState.Cached; - if (!CachedPlatformHasAllTagsPublished(srcPlatformData)) - { - cacheState = ImageCacheState.CachedWithMissingTags; - } - lock (_cachedPlatformsLock) - { - _cachedPlatforms[cacheKey] = srcPlatformData; - } - } - } - - return new ImageCacheResult(cacheState, isNewCacheHit, srcPlatformData); - } - - /// - /// Checks whether the source platform data has all its expected tags published. - /// - private static bool CachedPlatformHasAllTagsPublished(PlatformData srcPlatformData) => - (srcPlatformData.PlatformInfo?.Tags ?? []) - .Select(tag => tag.Name) - .AreEquivalent(srcPlatformData.SimpleTags); - - /// - /// Determines whether a previously published image can be reused by comparing the base image - /// digest and Dockerfile commit against the current state. - /// - private async Task CheckForCachedImageFromImageInfoAsync( - PlatformInfo platform, - PlatformData srcPlatformData, - ImageDigestCache imageDigestCache, - ImageNameResolver imageNameResolver, - string? sourceRepoUrl, - bool isLocalBaseImageExpected, - bool isDryRun) - { - _logger.LogInformation("Checking for cached image for '{DockerfilePath}'", platform.DockerfilePathRelativeToManifest); - - // If the previously published image was based on an image that is still the latest version AND - // the Dockerfile hasn't changed since it was last published - if (await IsBaseImageDigestUpToDateAsync( - platform, srcPlatformData, imageDigestCache, imageNameResolver, isLocalBaseImageExpected, isDryRun) && - IsDockerfileUpToDate(platform, srcPlatformData, sourceRepoUrl)) - { - return true; - } - - _logger.LogInformation("CACHE MISS"); - _logger.LogInformation(string.Empty); - - return false; - } - - /// - /// Checks whether the base image digest recorded in image-info matches the current digest - /// available from the registry. - /// - private async Task IsBaseImageDigestUpToDateAsync( - PlatformInfo platform, - PlatformData srcPlatformData, - ImageDigestCache imageDigestCache, - ImageNameResolver imageNameResolver, - bool isLocalImageExpected, - bool isDryRun) - { - _logger.LogInformation(string.Empty); - - if (platform.FinalStageFromImage is null) - { - _logger.LogInformation("Image does not have a base image. By default, it is considered up-to-date."); - return true; - } - - string queryImage = imageNameResolver.GetFinalStageImageNameForDigestQuery(platform); - - string? currentSha; - if (isLocalImageExpected) - { - currentSha = await imageDigestCache.GetLocalImageDigestAsync( - imageNameResolver.GetFromImageLocalTag(platform.FinalStageFromImage), isDryRun); - if (currentSha is not null) - { - currentSha = DockerHelper.GetDigestSha(currentSha); - } - } - else - { - try - { - currentSha = await imageDigestCache.GetManifestDigestShaAsync(queryImage, isDryRun); - } - // Handle cases where the image is not found in the registry yet. - // Other errors (e.g., authentication failures) should propagate so - // they are not silently swallowed. See https://github.com/dotnet/docker-tools/issues/1964 - catch (Exception ex) when (IsImageNotFoundException(ex)) - { - currentSha = null; - } - } - - string? imageInfoSha = srcPlatformData.BaseImageDigest is not null ? - DockerHelper.GetDigestSha(srcPlatformData.BaseImageDigest) : - null; - - bool baseImageDigestMatches = imageInfoSha?.Equals(currentSha, StringComparison.OrdinalIgnoreCase) == true; - - _logger.LogInformation("Image info's base image digest SHA: {ImageInfoSha}", imageInfoSha); - _logger.LogInformation("Latest base image digest SHA: {CurrentSha}", currentSha); - _logger.LogInformation("Base image digests match: {BaseImageDigestMatches}", baseImageDigestMatches); - return baseImageDigestMatches; - } - - /// - /// Checks whether the Dockerfile has changed since the last published build by comparing - /// the current git commit URL against the one recorded in image-info. - /// - private bool IsDockerfileUpToDate(PlatformInfo platform, PlatformData srcPlatformData, string? sourceRepoUrl) - { - string currentCommitUrl = _gitService.GetDockerfileCommitUrl(platform, sourceRepoUrl); - bool commitShaMatches = false; - if (srcPlatformData.CommitUrl is not null) - { - commitShaMatches = srcPlatformData.CommitUrl.Equals(currentCommitUrl, StringComparison.OrdinalIgnoreCase); - } - - _logger.LogInformation(string.Empty); - _logger.LogInformation("Image info's Dockerfile commit: {CommitUrl}", srcPlatformData.CommitUrl); - _logger.LogInformation("Latest Dockerfile commit: {CurrentCommitUrl}", currentCommitUrl); - _logger.LogInformation("Dockerfile commits match: {CommitShaMatches}", commitShaMatches); - return commitShaMatches; - } - - /// - /// Builds a cache key that uniquely identifies a platform build based on its Dockerfile path - /// and build arguments. - /// - private static string GetBuildCacheKey(PlatformInfo platform) => - $"{platform.DockerfilePathRelativeToManifest}-" + - string.Join('-', platform.BuildArgs.Select(kvp => $"{kvp.Key}={kvp.Value}").ToArray()); - - /// - /// Returns true if the exception represents an HTTP 404 Not Found response, - /// indicating the image does not exist in the registry. - /// - private static bool IsImageNotFoundException(Exception ex) => - (ex is HttpRequestException { StatusCode: HttpStatusCode.NotFound }) || - (ex is RequestFailedException { Status: 404 }); -} - -[Flags] -public enum ImageCacheState -{ - /// - /// Indicates a previously built image was not found in the registry. - /// - NotCached = 0, - - /// - /// Indicates a previously built image was found in the registry. - /// - Cached = 1, - - /// - /// Indicates a previously built image was found in the registry but is missing new tags. - /// - CachedWithMissingTags = Cached | 2 -} - -/// -/// The result of checking whether an image is cached. -/// -/// The cache state of the image. -/// Whether this is a newly discovered cache hit (not previously known in this session). -/// The source platform data associated with the cached image, if available. -public record ImageCacheResult(ImageCacheState State, bool IsNewCacheHit, PlatformData? Platform); diff --git a/src/ImageBuilder/ImageDigestCache.cs b/src/ImageBuilder/ImageDigestCache.cs deleted file mode 100644 index 327675576..000000000 --- a/src/ImageBuilder/ImageDigestCache.cs +++ /dev/null @@ -1,45 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; - -namespace Microsoft.DotNet.ImageBuilder -{ - public class ImageDigestCache(Lazy manifestService) - { - private readonly Lazy _inner = manifestService; - private readonly Dictionary _localDigestCache = []; - private readonly Dictionary _manifestDigestCache = []; - private readonly SemaphoreSlim _localDigestCacheLock = new(1); - private readonly SemaphoreSlim _manifestDigestCacheLock = new(1); - - public void AddDigest(string tag, string digest) - { - _localDigestCacheLock.Wait(); - try - { - _localDigestCache[tag] = digest; - } - finally - { - _localDigestCacheLock.Release(); - } - } - - public Task GetLocalImageDigestAsync(string tag, bool isDryRun) => - LockHelper.DoubleCheckedLockLookupAsync(_localDigestCacheLock, _localDigestCache, tag, - () => _inner.Value.GetLocalImageDigestAsync(tag, isDryRun), - // Don't allow null digests to be cached. A locally built image won't have a digest until - // it is pushed so if its digest is retrieved before pushing, we don't want that - // null to be cached. - val => !string.IsNullOrEmpty(val)); - - public Task GetManifestDigestShaAsync(string tag, bool isDryRun) => - LockHelper.DoubleCheckedLockLookupAsync(_manifestDigestCacheLock, _manifestDigestCache, tag, - () => _inner.Value.GetManifestDigestShaAsync(tag, isDryRun)); - } -} From 18b447126989f55514ce35618adce6eadee4d053 Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Mon, 3 Aug 2026 14:39:29 -0700 Subject: [PATCH 2/9] Relax build planning collection contracts Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d9f1ac1-4713-44b4-a6ba-17cb095984f6 --- .../Build/BuildPlannerTests.cs | 16 ++++++++-------- .../GenerateBuildMatrixCommandTests.cs | 2 +- src/ImageBuilder/Build/BuildPlanner.cs | 19 +++++++++---------- src/ImageBuilder/Build/BuildPolicies.cs | 10 +++++----- src/ImageBuilder/Commands/BuildCommand.cs | 6 +++--- .../Commands/GenerateBuildMatrixCommand.cs | 2 +- .../Commands/GetStaleImagesCommand.cs | 5 ++--- 7 files changed, 29 insertions(+), 31 deletions(-) diff --git a/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs b/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs index 4110b8d57..2c3fa7ff7 100644 --- a/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs +++ b/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs @@ -62,7 +62,7 @@ public async Task ChangedBaseImageExplainsFullDependencyChain() false)) .ReturnsAsync("sha256:new"); - IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + BuildPlanItem[] plan = await CreatePlanner().CreatePlanAsync( graph, imageInfo, CompositeBuildPolicy.ImageCache( @@ -113,7 +113,7 @@ public async Task ChangedTagSetsRequireReuse() BuildGraph graph = BuildGraph.Create(manifest); Mock manifestService = CreateDigestService("sha256:base"); - IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + BuildPlanItem[] plan = await CreatePlanner().CreatePlanAsync( graph, imageInfo, CompositeBuildPolicy.ImageCache( @@ -150,7 +150,7 @@ public async Task InvalidatedSharedBuildForcesEveryTargetToBuild() BuildGraph graph = BuildGraph.Create(manifest); Mock manifestService = CreateDigestService("sha256:new"); - IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + BuildPlanItem[] plan = await CreatePlanner().CreatePlanAsync( graph, imageInfo, CompositeBuildPolicy.ImageCache( @@ -190,7 +190,7 @@ public async Task SharedTagCreatesDependencyEdge() BuildGraph graph = BuildGraph.Create(manifest); Mock manifestService = CreateDigestService("sha256:new"); - IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + BuildPlanItem[] plan = await CreatePlanner().CreatePlanAsync( graph, imageInfo, CompositeBuildPolicy.ImageCache( @@ -267,7 +267,7 @@ public async Task CachedSiblingIsNotIncludedWhenAnotherChildBuilds() BuildGraph graph = BuildGraph.Create(manifest); Mock manifestService = CreateDigestService("sha256:base"); - IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + BuildPlanItem[] plan = await CreatePlanner().CreatePlanAsync( graph, imageInfo, CompositeBuildPolicy.ImageCache( @@ -289,7 +289,7 @@ public async Task CustomRuleMethodCanAddAPlanningDecision() CreateRepo("runtime", CreateImage(CreatePlatform( CreateDockerfile("runtime", tempFolder, "base:tag"), ["tag"]))))); BuildGraph graph = BuildGraph.Create(manifest); - IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + BuildPlanItem[] plan = await CreatePlanner().CreatePlanAsync( graph, imageInfo: null, new CompositeBuildPolicy( @@ -329,7 +329,7 @@ public async Task CompositePolicyAppliesEveryChildAndChoosesStrongestAction() BuildAction.BuildImage, new BuildReason("Build the image.")))); - IReadOnlyList plan = await CreatePlanner().CreatePlanAsync( + BuildPlanItem[] plan = await CreatePlanner().CreatePlanAsync( graph, CreatePublishedImages( manifest, @@ -389,7 +389,7 @@ private static Mock CreateDigestService(string digest) } private static BuildPlanItem GetItem( - IReadOnlyList plan, + IEnumerable plan, string repoName) => plan.Single(item => item.Target.Repo.Name == repoName); diff --git a/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs b/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs index 3d3306373..3fbb487a5 100644 --- a/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs +++ b/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs @@ -297,7 +297,7 @@ public async Task FilterOutCachedImages( ImageArtifactDetails imageInfo, IBuildPolicy policy, CancellationToken _) => Task.FromResult( - (IReadOnlyList)graph.Targets.Select(target => + graph.Targets.Select(target => new BuildPlanItem( target, actions.GetValueOrDefault( diff --git a/src/ImageBuilder/Build/BuildPlanner.cs b/src/ImageBuilder/Build/BuildPlanner.cs index aa7708776..8a5fcafc9 100644 --- a/src/ImageBuilder/Build/BuildPlanner.cs +++ b/src/ImageBuilder/Build/BuildPlanner.cs @@ -20,7 +20,7 @@ public class BuildPlanner(ILogger logger) private readonly ILogger _logger = logger ?? throw new ArgumentNullException(nameof(logger)); - public virtual async Task> CreatePlanAsync( + public virtual async Task CreatePlanAsync( BuildGraph graph, ImageArtifactDetails? imageInfo, IBuildPolicy policy, @@ -77,8 +77,7 @@ private static Dictionary CreatePublishedImageIndex .SelectMany(image => image.Platforms.Select(platform => ( Platform: platform, - SharedTags: (IReadOnlyList) - (image.Manifest?.SharedTags?.ToArray() ?? [])))) + SharedTags: image.Manifest?.SharedTags?.ToArray() ?? []))) .Where(item => item.Platform.PlatformInfo is not null && targetsByPlatform.ContainsKey(item.Platform.PlatformInfo)) @@ -149,10 +148,10 @@ private static async Task EvaluateAsync( hasPublishedImage ? publishedImage : null); } - private static IReadOnlyList> + private static List> GetSharedBuildsInDependencyOrder(BuildGraph graph) { - IReadOnlyList> sharedBuilds = graph.SharedBuildTargets + IReadOnlyList[] sharedBuilds = graph.SharedBuildTargets .Values .DistinctBy(targets => targets[0]) .ToArray(); @@ -205,7 +204,7 @@ void Visit(IReadOnlyList sharedBuild) private static void PropagateBuildsFromParents( BuildGraph graph, - IReadOnlyList sharedBuild, + IEnumerable sharedBuild, IDictionary items) { foreach (BuildTarget target in sharedBuild) @@ -229,7 +228,7 @@ private static void PropagateBuildsFromParents( } private static void UnifySharedBuildActions( - IReadOnlyList sharedBuild, + IEnumerable sharedBuild, IDictionary items) { BuildPlanItem? invalidatedItem = sharedBuild @@ -306,7 +305,7 @@ private static void UsePublishedParentsForBuilds( private static BuildPlanItem CreateItem( BuildTarget target, BuildAction action, - IReadOnlyList reasons, + IEnumerable reasons, PublishedImage? publishedImage) { if ((action is BuildAction.UsePublishedImage or BuildAction.PublishExistingImage) && @@ -317,10 +316,10 @@ private static BuildPlanItem CreateItem( "without a published image."); } - return new(target, action, reasons, publishedImage); + return new(target, action, reasons.ToArray(), publishedImage); } - private void LogPlan(IReadOnlyList plan) + private void LogPlan(IEnumerable plan) { foreach (BuildPlanItem item in plan) { diff --git a/src/ImageBuilder/Build/BuildPolicies.cs b/src/ImageBuilder/Build/BuildPolicies.cs index 53a506d0d..10891429d 100644 --- a/src/ImageBuilder/Build/BuildPolicies.cs +++ b/src/ImageBuilder/Build/BuildPolicies.cs @@ -50,11 +50,11 @@ Task EvaluateAsync( public sealed class CompositeBuildPolicy( BuildAction defaultAction, BuildReason defaultReason, - params IReadOnlyList policies) : IBuildPolicy + params IEnumerable policies) : IBuildPolicy { public static CompositeBuildPolicy ImageCache( BuildAction validImageAction, - params IReadOnlyList checks) => + params IEnumerable checks) => new( validImageAction, new( @@ -78,7 +78,7 @@ public async Task EvaluateAsync( .DefaultIfEmpty(BuildAction.NoAction) .Max(); BuildAction action = (BuildAction)Math.Max((int)childAction, (int)defaultAction); - IReadOnlyList reasons = results + BuildReason[] reasons = results .SelectMany(result => result.Reasons) .ToArray(); @@ -136,11 +136,11 @@ public Task EvaluateAsync( string[] expectedSharedTags = context.Target.Image.SharedTags .Select(tag => tag.Name) .ToArray(); - IReadOnlyList publishedPlatformTags = + IEnumerable publishedPlatformTags = publishedImage.Source == context.Target ? publishedImage.Image.SimpleTags : []; - IReadOnlyList publishedSharedTags = + IEnumerable publishedSharedTags = publishedImage.Source == context.Target ? publishedImage.SharedTags : []; diff --git a/src/ImageBuilder/Commands/BuildCommand.cs b/src/ImageBuilder/Commands/BuildCommand.cs index cd6070ffc..9109be2a5 100644 --- a/src/ImageBuilder/Commands/BuildCommand.cs +++ b/src/ImageBuilder/Commands/BuildCommand.cs @@ -111,7 +111,7 @@ public override async Task ExecuteAsync() skipManifestValidation: true); _buildGraph = BuildGraph.CreateFiltered(Manifest); await ExecuteWithDockerCredentialsAsync(() => PullBaseImagesAsync(_buildGraph)); - IReadOnlyList plan = await CreateBuildPlanAsync( + BuildPlanItem[] plan = await CreateBuildPlanAsync( _buildGraph, publishedImages); await BuildImagesAsync(plan); @@ -318,7 +318,7 @@ private async Task SetPlatformDataDigestAsync(PlatformData platform, string tag) platform.Digest = digest; } - private Task> CreateBuildPlanAsync( + private Task CreateBuildPlanAsync( BuildGraph graph, ImageArtifactDetails? publishedImages) { @@ -340,7 +340,7 @@ private Task> CreateBuildPlanAsync( policy); } - private async Task BuildImagesAsync(IReadOnlyList plan) + private async Task BuildImagesAsync(IEnumerable plan) { _logger.LogInformation("BUILDING IMAGES"); diff --git a/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs b/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs index eca05ef91..720c9861d 100644 --- a/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs +++ b/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs @@ -442,7 +442,7 @@ private async Task> GetPlatformsAsync() _gitService, Options.SourceRepoUrl ?? string.Empty)) : new AlwaysBuildPolicy("The image was selected for a build."); - IReadOnlyList plan = await _buildPlanner.CreatePlanAsync( + BuildPlanItem[] plan = await _buildPlanner.CreatePlanAsync( graph, imageInfo, policy); diff --git a/src/ImageBuilder/Commands/GetStaleImagesCommand.cs b/src/ImageBuilder/Commands/GetStaleImagesCommand.cs index 1e68e256c..edf095f7f 100644 --- a/src/ImageBuilder/Commands/GetStaleImagesCommand.cs +++ b/src/ImageBuilder/Commands/GetStaleImagesCommand.cs @@ -103,7 +103,7 @@ private async Task> GetPathsToRebuildAsync(Models.Subscripti BuildGraph graph = BuildGraph.CreateFiltered(manifest); string sourceRepoUrl = $"https://github.com/{subscription.Manifest.Owner}/{subscription.Manifest.Repo}"; - IReadOnlyList plan = await _buildPlanner.CreatePlanAsync( + BuildPlanItem[] plan = await _buildPlanner.CreatePlanAsync( graph, imageArtifactDetails, CompositeBuildPolicy.ImageCache( @@ -117,8 +117,7 @@ private async Task> GetPathsToRebuildAsync(Models.Subscripti return plan .Where(item => item.Action != BuildAction.NoAction) .Select(item => item.Target.Platform.Model.Dockerfile) - .Distinct() - .ToArray(); + .Distinct(); } private async Task GetImageInfoForSubscriptionAsync(Models.Subscription.Subscription subscription, ManifestInfo manifest) From e071ae8ef044b8d1c0e425b917309b62956f5879 Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Wed, 5 Aug 2026 13:31:39 -0700 Subject: [PATCH 3/9] Clean up code --- .../Build/BuildPlannerTests.cs | 35 +++-- src/ImageBuilder/Build/BuildPolicies.cs | 23 +-- src/ImageBuilder/Commands/BuildCommand.cs | 138 +++++++++--------- .../Commands/GenerateBuildMatrixCommand.cs | 68 ++++++--- .../Commands/GetStaleImagesCommand.cs | 30 +++- 5 files changed, 165 insertions(+), 129 deletions(-) diff --git a/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs b/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs index 2c3fa7ff7..482486a05 100644 --- a/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs +++ b/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs @@ -65,9 +65,12 @@ public async Task ChangedBaseImageExplainsFullDependencyChain() BuildPlanItem[] plan = await CreatePlanner().CreatePlanAsync( graph, imageInfo, - CompositeBuildPolicy.ImageCache( + new CompositeBuildPolicy( BuildAction.NoAction, - CreateBaseImageRule(manifest, manifestService.Object))); + new("All checks passed, so no work is required."), + new MissingPublishedImagePolicy(), + CreateBaseImageRule(manifest, manifestService.Object), + new TagSetChangedPolicy())); BuildPlanItem root = GetItem(plan, "root"); BuildPlanItem middle = GetItem(plan, "middle"); @@ -116,9 +119,12 @@ public async Task ChangedTagSetsRequireReuse() BuildPlanItem[] plan = await CreatePlanner().CreatePlanAsync( graph, imageInfo, - CompositeBuildPolicy.ImageCache( + new CompositeBuildPolicy( BuildAction.NoAction, - CreateBaseImageRule(manifest, manifestService.Object))); + new("All checks passed, so no work is required."), + new MissingPublishedImagePolicy(), + CreateBaseImageRule(manifest, manifestService.Object), + new TagSetChangedPolicy())); BuildPlanItem item = plan.ShouldHaveSingleItem(); item.Action.ShouldBe(BuildAction.PublishExistingImage); @@ -153,9 +159,12 @@ public async Task InvalidatedSharedBuildForcesEveryTargetToBuild() BuildPlanItem[] plan = await CreatePlanner().CreatePlanAsync( graph, imageInfo, - CompositeBuildPolicy.ImageCache( + new CompositeBuildPolicy( BuildAction.NoAction, - CreateBaseImageRule(manifest, manifestService.Object))); + new("All checks passed, so no work is required."), + new MissingPublishedImagePolicy(), + CreateBaseImageRule(manifest, manifestService.Object), + new TagSetChangedPolicy())); BuildPlanItem first = GetItem(plan, "first"); BuildPlanItem second = GetItem(plan, "second"); @@ -193,9 +202,12 @@ public async Task SharedTagCreatesDependencyEdge() BuildPlanItem[] plan = await CreatePlanner().CreatePlanAsync( graph, imageInfo, - CompositeBuildPolicy.ImageCache( + new CompositeBuildPolicy( BuildAction.NoAction, - CreateBaseImageRule(manifest, manifestService.Object))); + new("All checks passed, so no work is required."), + new MissingPublishedImagePolicy(), + CreateBaseImageRule(manifest, manifestService.Object), + new TagSetChangedPolicy())); BuildPlanItem child = GetItem(plan, "child"); child.Action.ShouldBe(BuildAction.BuildImage); @@ -270,9 +282,12 @@ public async Task CachedSiblingIsNotIncludedWhenAnotherChildBuilds() BuildPlanItem[] plan = await CreatePlanner().CreatePlanAsync( graph, imageInfo, - CompositeBuildPolicy.ImageCache( + new CompositeBuildPolicy( BuildAction.NoAction, - CreateBaseImageRule(manifest, manifestService.Object))); + new("All checks passed, so no work is required."), + new MissingPublishedImagePolicy(), + CreateBaseImageRule(manifest, manifestService.Object), + new TagSetChangedPolicy())); GetItem(plan, "parent").Action.ShouldBe(BuildAction.UsePublishedImage); GetItem(plan, "first").Action.ShouldBe(BuildAction.BuildImage); diff --git a/src/ImageBuilder/Build/BuildPolicies.cs b/src/ImageBuilder/Build/BuildPolicies.cs index 10891429d..893220890 100644 --- a/src/ImageBuilder/Build/BuildPolicies.cs +++ b/src/ImageBuilder/Build/BuildPolicies.cs @@ -52,39 +52,26 @@ public sealed class CompositeBuildPolicy( BuildReason defaultReason, params IEnumerable policies) : IBuildPolicy { - public static CompositeBuildPolicy ImageCache( - BuildAction validImageAction, - params IEnumerable checks) => - new( - validImageAction, - new( - validImageAction == BuildAction.UsePublishedImage - ? "All checks passed, so this invocation will use the published image." - : "All checks passed, so no work is required."), - [ - new MissingPublishedImagePolicy(), - ..checks, - new TagSetChangedPolicy() - ]); - public async Task EvaluateAsync( BuildPolicyContext context, CancellationToken cancellationToken = default) { + BuildPolicyResult[] results = await Task.WhenAll( policies.Select(policy => policy.EvaluateAsync(context, cancellationToken))); + BuildAction childAction = results .Select(result => result.Action) .DefaultIfEmpty(BuildAction.NoAction) .Max(); + BuildAction action = (BuildAction)Math.Max((int)childAction, (int)defaultAction); + BuildReason[] reasons = results .SelectMany(result => result.Reasons) .ToArray(); - return new( - action, - childAction == BuildAction.NoAction ? [..reasons, defaultReason] : reasons); + return new BuildPolicyResult(action, childAction == BuildAction.NoAction ? [..reasons, defaultReason] : reasons); } } diff --git a/src/ImageBuilder/Commands/BuildCommand.cs b/src/ImageBuilder/Commands/BuildCommand.cs index 9109be2a5..4bed7ed46 100644 --- a/src/ImageBuilder/Commands/BuildCommand.cs +++ b/src/ImageBuilder/Commands/BuildCommand.cs @@ -109,11 +109,13 @@ public override async Task ExecuteAsync() Options.ImageInfoSourcePath, Manifest, skipManifestValidation: true); + _buildGraph = BuildGraph.CreateFiltered(Manifest); + await ExecuteWithDockerCredentialsAsync(() => PullBaseImagesAsync(_buildGraph)); - BuildPlanItem[] plan = await CreateBuildPlanAsync( - _buildGraph, - publishedImages); + + BuildPlanItem[] plan = await CreateBuildPlanAsync(_buildGraph, publishedImages); + await BuildImagesAsync(plan); if (_processedTags.Count > 0 || _hasPublishedImagesToUse) @@ -178,6 +180,7 @@ private async Task PublishImageInfoAsync() Dictionary platformDataByPlatform = processedPlatforms .Where(platform => platform.PlatformInfo is not null) .ToDictionary(platform => platform.PlatformInfo!); + List platformsWithNoPushTags = new List(); foreach (PlatformData platform in processedPlatforms) @@ -246,19 +249,15 @@ private void SetPlatformDataBaseDigest( string? baseImageDigest = platform.BaseImageDigest; if (platform.BaseImageDigest is null && platform.PlatformInfo?.FinalStageFromImage is not null) { - BuildGraph graph = _buildGraph ?? - throw new InvalidOperationException("Build graph has not been created."); - BuildTarget target = graph.Targets.First( - target => target.Platform == platform.PlatformInfo); - BuildTarget? parent = graph.Parents[target].SingleOrDefault(parent => - parent.Platform.Tags + BuildGraph graph = _buildGraph ?? throw new InvalidOperationException("Build graph has not been created."); + BuildTarget target = graph.Targets.First(target => target.Platform == platform.PlatformInfo); + + BuildTarget? parent = graph.Parents[target].SingleOrDefault( + parent => parent.Platform.Tags .Concat(parent.Image.SharedTags) - .Any(tag => - tag.FullyQualifiedName == platform.PlatformInfo.FinalStageFromImage)); - if (parent is null || - !platformDataByPlatform.TryGetValue( - parent.Platform, - out PlatformData? basePlatformData)) + .Any(tag => tag.FullyQualifiedName == platform.PlatformInfo.FinalStageFromImage)); + + if (parent is null || !platformDataByPlatform.TryGetValue(parent.Platform, out PlatformData? basePlatformData)) { throw new InvalidOperationException( $"Unable to find platform data for tag '{platform.PlatformInfo.FinalStageFromImage}'. " + @@ -318,48 +317,55 @@ private async Task SetPlatformDataDigestAsync(PlatformData platform, string tag) platform.Digest = digest; } - private Task CreateBuildPlanAsync( - BuildGraph graph, - ImageArtifactDetails? publishedImages) + private Task CreateBuildPlanAsync(BuildGraph graph, ImageArtifactDetails? publishedImages) { IBuildPolicy policy = Options.NoCache ? new AlwaysBuildPolicy() - : CompositeBuildPolicy.ImageCache( - BuildAction.UsePublishedImage, - BaseImageChangedPolicy.FromLocalImages( - _imageDigestCache, - _imageNameResolver.Value, - Options.IsDryRun), - new DockerfileChangedPolicy( - _gitService, - Options.SourceRepoUrl ?? string.Empty)); - - return _buildPlanner.CreatePlanAsync( - graph, - publishedImages, - policy); + : new CompositeBuildPolicy( + defaultAction: BuildAction.UsePublishedImage, + defaultReason: new BuildReason("All checks passed, so this invocation will use the published image."), + policies: + [ + // Rebuild when no published image metadata exists. + new MissingPublishedImagePolicy(), + + // Rebuild when the locally available base image digest has changed. + BaseImageChangedPolicy.FromLocalImages( + _imageDigestCache, + _imageNameResolver.Value, + Options.IsDryRun), + + // Rebuild when the Dockerfile has changed. + new DockerfileChangedPolicy( + _gitService, + sourceRepoUrl: Options.SourceRepoUrl ?? string.Empty), + + // Republish the existing image when its configured tags have changed. + new TagSetChangedPolicy() + ]); + + return _buildPlanner.CreatePlanAsync(graph, publishedImages, policy); } private async Task BuildImagesAsync(IEnumerable plan) { _logger.LogInformation("BUILDING IMAGES"); - BuildPlanItem[] executableItems = plan - .Where(item => item.Action != BuildAction.NoAction) - .ToArray(); + BuildPlanItem[] executableItems = plan.Where(item => item.Action != BuildAction.NoAction) .ToArray(); + _hasPublishedImagesToUse = executableItems.Any( - item => item.Action is - BuildAction.UsePublishedImage or - BuildAction.PublishExistingImage); + item => item.Action + is BuildAction.UsePublishedImage + or BuildAction.PublishExistingImage); - foreach (IGrouping repoPlan in executableItems - .GroupBy(item => item.Target.Repo)) + var repoPlans = executableItems.GroupBy(item => item.Target.Repo); + foreach (IGrouping repoPlan in repoPlans) { RepoInfo repoInfo = repoPlan.Key; RepoData repoData = CreateRepoData(repoInfo); - foreach (IGrouping imagePlan in repoPlan - .GroupBy(item => item.Target.Image)) + var imagePlans = repoPlan.GroupBy(item => item.Target.Image); + foreach (IGrouping imagePlan in imagePlans) { ImageInfo image = imagePlan.Key; ImageData imageData = CreateImageData(image); @@ -383,21 +389,16 @@ BuildAction.UsePublishedImage or PlatformData platformData = CreatePlatformData(image, platform); imageData.Platforms.Add(platformData); - if (plannedImage.Action is - BuildAction.UsePublishedImage or - BuildAction.PublishExistingImage) + if (plannedImage.Action is BuildAction.UsePublishedImage or BuildAction.PublishExistingImage) { PublishedImage publishedImage = plannedImage.PublishedImage ?? throw new InvalidOperationException( $"Build plan did not provide reusable metadata for '{platform.DockerfilePath}'."); CopyPlatformDataFromCachedPlatform(platformData, publishedImage.Image); - platformData.IsUnchanged = - plannedImage.Action == BuildAction.UsePublishedImage; - await UsePublishedImageAsync( - repoInfo, - allTagInfos, - publishedImage.Image.Digest); + platformData.IsUnchanged = plannedImage.Action == BuildAction.UsePublishedImage; + + await UsePublishedImageAsync(repoInfo, allTagInfos, publishedImage.Image.Digest); } else if (plannedImage.Action == BuildAction.BuildImage) { @@ -409,8 +410,9 @@ await UsePublishedImageAsync( if (Options.IsPushEnabled && platform.FinalStageFromImage is not null) { platformData.BaseImageDigest = - await _imageDigestCache.GetLocalImageDigestAsync( - _imageNameResolver.Value.GetFromImageLocalTag(platform.FinalStageFromImage), Options.IsDryRun); + await _imageDigestCache.GetLocalImageDigestAsync( + _imageNameResolver.Value.GetFromImageLocalTag(platform.FinalStageFromImage), + Options.IsDryRun); } } } @@ -423,25 +425,23 @@ await _imageDigestCache.GetLocalImageDigestAsync( } } - private void CopyPlatformDataFromCachedPlatform( - PlatformData destination, - PlatformData source) + private void CopyPlatformDataFromCachedPlatform(PlatformData destination, PlatformData source) { // When a cache hit occurs for a Dockerfile, we want to transfer some of the metadata about the previously // published image so we don't need to recalculate it again. destination.BaseImageDigest = source.BaseImageDigest; - destination.Layers = new List(source.Layers); + destination.Layers = [.. source.Layers]; } - private RepoData CreateRepoData(RepoInfo repoInfo) => - new RepoData - { - Repo = repoInfo.Name - }; + private RepoData CreateRepoData(RepoInfo repoInfo) => new RepoData + { + Repo = repoInfo.Name + }; private PlatformData CreatePlatformData(ImageInfo image, PlatformInfo platform) { PlatformData platformData = PlatformData.FromPlatformInfo(platform, image); + platformData.SimpleTags = platform.Tags .Select(tag => tag.Name) .OrderBy(name => name) @@ -452,11 +452,10 @@ private PlatformData CreatePlatformData(ImageInfo image, PlatformInfo platform) private ImageData CreateImageData(ImageInfo image) { - ImageData imageData = - new ImageData - { - ProductVersion = image.ProductVersion - }; + var imageData = new ImageData + { + ProductVersion = image.ProductVersion + }; if (image.SharedTags.Any()) { @@ -695,9 +694,8 @@ private async Task PullBaseImagesAsync(BuildGraph graph) HashSet pulledTags = []; HashSet externalFromImages = []; - PlatformInfo[] platforms = graph.Targets - .Select(target => target.Platform) - .ToArray(); + PlatformInfo[] platforms = graph.Targets.Select(target => target.Platform) .ToArray(); + foreach (PlatformInfo platform in platforms) { IEnumerable platformExternalFromImages = platform.ExternalFromImages.Distinct(); diff --git a/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs b/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs index 720c9861d..de82fce00 100644 --- a/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs +++ b/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs @@ -206,11 +206,12 @@ private IEnumerable GetCustomLegGroupPlatforms(PlatformInfo platfo { IEnumerable dependencyPlatforms = group.Dependencies .Select(dependency => Manifest.GetPlatformByTag(dependency)); - return dependencyPlatforms - .Concat(dependencyPlatforms - .SelectMany(dependencyPlatform => GetAncestors( - dependencyPlatform, - Manifest.GetFilteredPlatforms()))); + + return dependencyPlatforms.Concat( + dependencyPlatforms.SelectMany( + dependencyPlatform => GetAncestors(dependencyPlatform, Manifest.GetFilteredPlatforms()) + ) + ); }) .Distinct(); } @@ -417,10 +418,16 @@ private static string FormatMatrixName(IEnumerable parts) return allParts.First() + string.Join(string.Empty, allParts.Skip(1).Select(part => part.FirstCharToUpper())); } - private async Task> GetPlatformsAsync() + /// + /// Selects the manifest platforms that require build matrix legs. + /// + private async Task> GetPlatformsToBuildAsync() { + // Matrix grouping still needs dependency relationships from the complete manifest. _dependencyGraph = BuildGraph.Create(Manifest); ImageArtifactDetails? imageInfo = _imageArtifactDetails.Value; + + // Do not schedule platforms that an earlier build already completed unchanged. HashSet completedPlatforms = imageInfo?.Repos .SelectMany(repo => repo.Images) .SelectMany(image => image.Platforms) @@ -428,28 +435,42 @@ private async Task> GetPlatformsAsync() .Select(platform => platform.PlatformInfo!) .ToHashSet() ?? []; - BuildGraph graph = BuildGraph.CreateFiltered( - Manifest, - platform => !completedPlatforms.Contains(platform)); + + // Build a second, filtered dependency graph... + BuildGraph graph = BuildGraph.CreateFiltered(Manifest, platform => !completedPlatforms.Contains(platform)); + + // ...and then check which images actually need to be built. IBuildPolicy policy = Options.TrimCachedImages - ? CompositeBuildPolicy.ImageCache( - BuildAction.NoAction, - BaseImageChangedPolicy.FromRegistry( - _imageDigestCache, - _imageNameResolver.Value, - Options.IsDryRun), - new DockerfileChangedPolicy( - _gitService, - Options.SourceRepoUrl ?? string.Empty)) + ? new CompositeBuildPolicy( + defaultAction: BuildAction.NoAction, + defaultReason: new BuildReason("All checks passed, so no work is required."), + policies: + [ + // Rebuild when no published image metadata exists. + new MissingPublishedImagePolicy(), + + // Rebuild when the registry digest for the base image has changed. + BaseImageChangedPolicy.FromRegistry( + _imageDigestCache, + _imageNameResolver.Value, + Options.IsDryRun), + + // Rebuild when the Dockerfile has changed. + new DockerfileChangedPolicy( + _gitService, + sourceRepoUrl: Options.SourceRepoUrl ?? string.Empty), + + // Republish the existing image when its configured tags have changed. + new TagSetChangedPolicy() + ]) : new AlwaysBuildPolicy("The image was selected for a build."); - BuildPlanItem[] plan = await _buildPlanner.CreatePlanAsync( - graph, - imageInfo, - policy); + + BuildPlanItem[] plan = await _buildPlanner.CreatePlanAsync(graph, imageInfo, policy); IEnumerable plannedPlatforms = plan .Where(item => item.Action != BuildAction.NoAction) .Select(item => item.Target.Platform); + return Options.TrimCachedImages ? plannedPlatforms.OrderBy(platform => platform.DockerfilePath) : plannedPlatforms; @@ -502,7 +523,8 @@ public async Task> GenerateMatrixInfoAsync() List matrices = []; // The sort order used here is arbitrary and simply helps the readability of the output. - IOrderedEnumerable> platformGroups = (await GetPlatformsAsync()) + IOrderedEnumerable> platformGroups = + (await GetPlatformsToBuildAsync()) .GroupBy(platform => CreatePlatformId(platform)) .OrderBy(platformGroup => platformGroup.Key.OS) .ThenByDescending(platformGroup => platformGroup.Key.OsVersion) diff --git a/src/ImageBuilder/Commands/GetStaleImagesCommand.cs b/src/ImageBuilder/Commands/GetStaleImagesCommand.cs index edf095f7f..2c493c6ed 100644 --- a/src/ImageBuilder/Commands/GetStaleImagesCommand.cs +++ b/src/ImageBuilder/Commands/GetStaleImagesCommand.cs @@ -101,18 +101,32 @@ private async Task> GetPathsToRebuildAsync(Models.Subscripti sourceRepoPrefix: Options.SourceRepoPrefix); BuildGraph graph = BuildGraph.CreateFiltered(manifest); - string sourceRepoUrl = - $"https://github.com/{subscription.Manifest.Owner}/{subscription.Manifest.Repo}"; - BuildPlanItem[] plan = await _buildPlanner.CreatePlanAsync( - graph, - imageArtifactDetails, - CompositeBuildPolicy.ImageCache( - BuildAction.NoAction, + + IBuildPolicy policy = new CompositeBuildPolicy( + defaultAction: BuildAction.NoAction, + defaultReason: new BuildReason("All checks passed, so no work is required."), + policies: + [ + // Rebuild when no published image metadata exists. + new MissingPublishedImagePolicy(), + + // Rebuild when the registry digest for the base image has changed. BaseImageChangedPolicy.FromRegistry( _imageDigestCache, imageNameResolver, Options.IsDryRun), - new DockerfileChangedPolicy(_gitService, sourceRepoUrl))); + + // Rebuild when the Dockerfile has changed. + new DockerfileChangedPolicy( + _gitService, + sourceRepoUrl: + $"https://github.com/{subscription.Manifest.Owner}/{subscription.Manifest.Repo}"), + + // Republish the existing image when its configured tags have changed. + new TagSetChangedPolicy() + ]); + + BuildPlanItem[] plan = await _buildPlanner.CreatePlanAsync(graph, imageArtifactDetails, policy); return plan .Where(item => item.Action != BuildAction.NoAction) From 2af0e5702d1ea0fe08779769bb4262d24e40d3ea Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Wed, 5 Aug 2026 14:44:06 -0700 Subject: [PATCH 4/9] Revert documentation changes for now --- documentation/base-image-dependency-flow.md | 4 +- eng/docker-tools/CHANGELOG.md | 21 -------- eng/docker-tools/DEV-GUIDE.md | 54 +++++++-------------- 3 files changed, 19 insertions(+), 60 deletions(-) diff --git a/documentation/base-image-dependency-flow.md b/documentation/base-image-dependency-flow.md index 853c89119..be02778d7 100644 --- a/documentation/base-image-dependency-flow.md +++ b/documentation/base-image-dependency-flow.md @@ -18,8 +18,8 @@ Another part of the automation is an Azure DevOps pipeline that will check wheth For each subscription described in the subscriptions file, the pipeline runs the following process: 1. Loads the manifest and Dockerfile from the subscribed repo. The manifest contains metadata about all the images that are to be produced for the repo, how they are to be tagged, and a bunch of other information. This file, when paired with the content of the repo's Dockerfiles, provides an easily consumable description of the base image tags that are being depended upon. 2. Now that we've got the list of base image tags that we care about, the pipeline makes use of the Docker Registry to get the latest digest values for those tags. -3. Remember that [image info](https://github.com/dotnet/versions/blob/main/build-info/docker/image-info.dotnet-dotnet-docker-main.json) file described above? This is where it comes in handy. ImageBuilder creates a build plan by comparing each recorded final-stage base image digest to the latest digest retrieved in the previous step. A changed digest produces a `BuildImage` action. That action propagates to every dependent image, and other parents required by those dependents are included in the plan. Each action records its cause, so a dependent image can be traced back through the dependency chain to the base image that changed. -4. If the plan contains work, the pipeline constructs a special build argument that describes the actionable Dockerfile paths. It then queues a build for the pipeline referenced by the subscription in the subscriptions file, passing it this build argument. When that pipeline builds the images, it will ensure that the built images are making use of the latest base images, just by the nature of the build process pulling the base image before building. You can see here that there's a virtuous cycle; when the pipeline publishes these updated images, it updates the main image info so that it now references the latest base image digests. That is, until the next base image changes and the process starts all over again. +3. Remember that [image info](https://github.com/dotnet/versions/blob/main/build-info/docker/image-info.dotnet-dotnet-docker-main.json) file described above? This is where it comes in handy. The pipeline reads that file to determine which digests the published images are dependent upon and compares them to the latest digests that were retrieved in the previous step. If those values are different, it means the published image is making use of an older version of the image. We need to get it updated! The pipeline keeps track of which images, and their dependent images, need to be rebuilt. +4. If any images need to be rebuilt, the pipeline constructs a special build argument that describes the set of paths to the Dockerfiles that need to be built. It then queues a build for the pipeline referenced by the subscription in the subscriptions file, passing it this build argument. When that pipeline builds the images, it will ensure that the built images are making use of the latest base images, just by the nature of the build process pulling the base image before building. You can see here that there's a virtuous cycle; when the pipeline publishes these updated images, it updates the main image info so that it now references the latest base image digests. That is, until the next base image changes and the process starts all over again. This entire process is hands-off automation running 7 days a week, allowing consumers of .NET Docker images to be assured they're getting the most up-to-date version of the images they care about. diff --git a/eng/docker-tools/CHANGELOG.md b/eng/docker-tools/CHANGELOG.md index f05f2e5b9..0936f6feb 100644 --- a/eng/docker-tools/CHANGELOG.md +++ b/eng/docker-tools/CHANGELOG.md @@ -4,27 +4,6 @@ All breaking changes and new features in `eng/docker-tools` will be documented i --- -## 2026-07-31: Centralized ImageBuilder build planning - -ImageBuilder now calculates one explainable build plan for matrix trimming, build execution, and -stale-image detection. Rebuilds propagate through internal image dependencies, and each planned -action records a readable causal chain back to the original cache invalidation. Cache checks -implement one `IBuildPolicy` contract and are composed through `CompositeBuildPolicy`, so new -invalidation sources can be added without changing the planner algorithm. - -Matrix generation, build execution, and stale-image detection now use the same cache rules for -missing image-info, base-image changes, Dockerfile commits, and tag-set changes. - -Cached images with changed platform or shared tags are now retained in the build matrix as -`PublishExistingImage` actions so tag additions, removals, and moves can be published without -rebuilding the image. Normal build configuration does not need to change, but these cases can -produce a build job where they were previously trimmed. - -Code that embeds ImageBuilder must replace `IImageCacheService` with `BuildPlanner`. CLI -arguments and pipeline parameters are unchanged. - ---- - ## 2026-06-11: Configurable per-registry referrer-lookup rate limit - Issue: [#2141](https://github.com/dotnet/docker-tools/issues/2141) diff --git a/eng/docker-tools/DEV-GUIDE.md b/eng/docker-tools/DEV-GUIDE.md index e7e9afbf8..d47fb3ebd 100644 --- a/eng/docker-tools/DEV-GUIDE.md +++ b/eng/docker-tools/DEV-GUIDE.md @@ -361,49 +361,27 @@ The `autobuilder` label is how the infrastructure tracks that the failure cycle --- -### Build Planning and Image Caching +### Image Caching -ImageBuilder calculates a build plan before deciding what work to run. Each platform receives one action: +The infrastructure includes caching to avoid rebuilding images that haven't changed. Caching operates at two levels: -- **BuildImage**: run the Docker build. -- **PublishExistingImage**: use the valid published image and continue it through downstream processing because tags or other published metadata changed. -- **UsePublishedImage**: the published image is valid, but this invocation must pull, import, or retag it. -- **NoAction**: the published image is valid and this invocation does not need it locally. +**1. Matrix Trimming (job-level caching)** -Each action includes readable reasons. When an image is rebuilt because a dependency changed, the reason links to that dependency's reason. This preserves the complete explanation from a dependent image back to the original cache invalidation. +When `trimCachedImagesForMatrix` is enabled, the `generateBuildMatrix` command excludes platforms from the build matrix if they would result in cache hits. This means no build job is even created for those platforms—they're completely skipped. -The same planner is used by: +**2. Build-time Caching** -1. **Matrix trimming**: `generateBuildMatrix` omits `NoAction` platforms when `trimCachedImagesForMatrix` is enabled. -2. **Build execution**: `build` executes `BuildImage` actions and materializes `PublishExistingImage` and `UsePublishedImage` actions. -3. **Stale-image detection**: `getStaleImages` uses the same checks to identify actionable paths before queueing a build. +Even if a platform isn't trimmed from the matrix, the `build` command checks each image against the cache before building. If the image is cached, it outputs `CACHE HIT`, pulls the previously-built image from the registry, and skips the actual Docker build. -Commands select the manifest-filtered graph. Every target in that graph is evaluated with the same cache rules; the planner owns the build decision and dependency propagation. +#### Cache Conditions -Planning code lives in `Microsoft.DotNet.ImageBuilder.Build`: +An image is considered cached when **both** of the following conditions are true: -- `BuildGraph` is the only graph abstraction. Its public dictionaries contain parent, child, and shared-build relationships for each target. Parent edges include platform tags and image-level shared tags. Targets share a build when they have the same Dockerfile, target platform, build arguments, and effective FROM overrides. -- `BuildTarget` represents the current desired definition and retains its `PlatformInfo`, image, and repo context for execution. -- `PlatformData` remains the mutable image-info and published-image model. `BuildPlanner` joins it to graph targets only while creating a plan and records the source target when equivalent targets reuse the same published image. -- `IBuildPolicy` is the only check/policy contract. Each check is a small policy class. `CompositeBuildPolicy` applies every child policy and combines their results, with `BuildImage` taking precedence over `PublishExistingImage`, then `UsePublishedImage`, then `NoAction`. -- The ordered `BuildPlanItem` sequence is the execution input. The build command groups those items by repo and image instead of traversing the manifest again to rediscover work. +1. **Base image digest is unchanged** — The digest of the base image (FROM image) matches the digest recorded in the image info file from the last successful publish. If the upstream base image has been updated, this condition fails and the image will be rebuilt. -Adding an invalidation source, such as package-version metadata or intermediate image dependencies, requires one `IBuildPolicy` implementation and adding it to the composite. Dependency and shared-build propagation remain inside `BuildPlanner`; there is no separate resolver. +2. **Dockerfile commit is unchanged** — The git commit URL for the Dockerfile matches the commit URL recorded in the image info file. If you've modified the Dockerfile, this condition fails and the image will be rebuilt. -#### Planning Rules - -A previously published image is valid when its final-stage base image digest and Dockerfile commit still match the current values. Missing image metadata, a changed base image, or a changed Dockerfile produces a `BuildImage` action. Any platform or image-level shared tag-set change produces a `PublishExistingImage` action so additions, removals, and moves can be published without rebuilding the image. Matrix generation, build execution, and stale-image detection all use this rule sequence. - -When a platform must be built, all descendants in the caller-selected graph are also built. Other parents needed by those descendants are included as `BuildImage` or `UsePublishedImage` actions. Platforms in the same shared-build group can share published image metadata; if one evaluated target in that group is invalidated, every evaluated target in the group is built. - -The planner traverses shared-build groups from graph roots to leaves. It evaluates targets -sequentially within each group, propagates parent `BuildImage` actions to the group, and then -unifies the group's actions. After all build decisions are final, unchanged direct parents -required by built images are changed from `NoAction` to `UsePublishedImage`. - -`getStaleImages`, matrix generation, and build execution evaluate the same cache checks. - -Planning compares against the published image info stored in the [versions repo](https://github.com/dotnet/versions). This means planning compares against what has been officially published, not only what is in the current branch. +Caching compares against the published image info stored in the [versions repo](https://github.com/dotnet/versions). This means caching compares against what's been officially published, not what's in your current branch. #### Disabling Caching @@ -514,14 +492,16 @@ If your Dockerfile path doesn't appear in any of the matrix legs, it was trimmed **How to fix:** Set the `noCache` parameter to `true` when queuing the build. -#### Symptom 3: The build output shows `USING PUBLISHED IMAGE` +#### Symptom 3: The build output shows `CACHE HIT` -If your build job runs but you see `USING PUBLISHED IMAGE` in the output of the `Build Images` step and the Dockerfile isn't actually built, build planning determined that the existing published image is valid but needed by this invocation. This is an example of what the output in that step looks like: +If your build job runs but you see `CACHE HIT` in the output of the `Build Images` step and the Dockerfile isn't actually built, the [build-time caching](#image-caching) determined that the image doesn't need to be rebuilt. This is an example of what the output in that step looks like: ``` -Build plan for src/windowsservercore/ltsc2025/helix/amd64/Dockerfile: UsePublishedImage. Base image 'mcr.microsoft.com/windows/servercore:ltsc2025' is unchanged at 'sha256:...'. Dockerfile is unchanged at 'https://github.com/dotnet/dotnet-buildtools-prereqs-docker/blob/.../Dockerfile'. +Image info's Dockerfile commit: https://github.com/dotnet/dotnet-buildtools-prereqs-docker/blob/aa85f0dcc3b3d6757c80dc8c2a6f38c290b372cc/src/windowsservercore/ltsc2025/helix/amd64/Dockerfile +Latest Dockerfile commit: https://github.com/dotnet/dotnet-buildtools-prereqs-docker/blob/aa85f0dcc3b3d6757c80dc8c2a6f38c290b372cc/src/windowsservercore/ltsc2025/helix/amd64/Dockerfile +Dockerfile commits match: True -USING PUBLISHED IMAGE +CACHE HIT -- EXECUTING: docker pull mcr.microsoft.com/dotnet-buildtools/prereqs@sha256:40d36a0aab610f4d513ed7c7300a5d962968a547ffe8a859a0e599691b74b77f ``` From 78dd4b6c17c0b8ae371d97dfd97cfc334f05f90d Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Thu, 6 Aug 2026 15:39:28 -0700 Subject: [PATCH 5/9] Only check base image and unpublished images in GetStaleImages --- .../GetStaleImagesCommandTests.cs | 21 ------------------- .../Commands/GetStaleImagesCommand.cs | 11 +--------- 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/src/ImageBuilder.Tests/GetStaleImagesCommandTests.cs b/src/ImageBuilder.Tests/GetStaleImagesCommandTests.cs index f1fe3e433..1096e2e43 100644 --- a/src/ImageBuilder.Tests/GetStaleImagesCommandTests.cs +++ b/src/ImageBuilder.Tests/GetStaleImagesCommandTests.cs @@ -1668,7 +1668,6 @@ private static Subscription CreateSubscription( /// private class TestFixture : IDisposable { - private const string DockerfileCommitSha = "current-commit"; private readonly List filesToCleanup = new List(); private readonly List foldersToCleanup = new List(); private readonly Dictionary imageDigests = new Dictionary(); @@ -1701,21 +1700,6 @@ public TestFixture( string osType = "*") { this.osType = osType; - foreach (SubscriptionInfo subscriptionInfo in subscriptionInfos) - { - string sourceRepoUrl = - $"https://github.com/{subscriptionInfo.Subscription.Manifest.Owner}/" + - subscriptionInfo.Subscription.Manifest.Repo; - foreach (PlatformData platform in subscriptionInfo.ImageInfo.Repos - .SelectMany(repo => repo.Images) - .SelectMany(image => image.Platforms)) - { - platform.CommitUrl = - $"{sourceRepoUrl}/blob/{DockerfileCommitSha}/" + - PathHelper.NormalizePath(platform.Dockerfile); - } - } - this.subscriptionsPath = this.SerializeJsonObjectToTempFile( subscriptionInfos.Select(tuple => tuple.Subscription).ToArray()); @@ -1881,11 +1865,6 @@ private IGitService CreateGitService( Dictionary> dockerfileInfos) { Mock gitServiceMock = new(); - gitServiceMock - .Setup(service => service.GetCommitSha( - It.IsAny(), - useFullHash: true)) - .Returns(DockerfileCommitSha); foreach (SubscriptionInfo subscriptionInfo in subscriptionInfos) { diff --git a/src/ImageBuilder/Commands/GetStaleImagesCommand.cs b/src/ImageBuilder/Commands/GetStaleImagesCommand.cs index 2c493c6ed..745238a2b 100644 --- a/src/ImageBuilder/Commands/GetStaleImagesCommand.cs +++ b/src/ImageBuilder/Commands/GetStaleImagesCommand.cs @@ -114,16 +114,7 @@ private async Task> GetPathsToRebuildAsync(Models.Subscripti BaseImageChangedPolicy.FromRegistry( _imageDigestCache, imageNameResolver, - Options.IsDryRun), - - // Rebuild when the Dockerfile has changed. - new DockerfileChangedPolicy( - _gitService, - sourceRepoUrl: - $"https://github.com/{subscription.Manifest.Owner}/{subscription.Manifest.Repo}"), - - // Republish the existing image when its configured tags have changed. - new TagSetChangedPolicy() + Options.IsDryRun) ]); BuildPlanItem[] plan = await _buildPlanner.CreatePlanAsync(graph, imageArtifactDetails, policy); From 31328ecb012edf20d49136e50ad413158dba0367 Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Thu, 6 Aug 2026 16:13:01 -0700 Subject: [PATCH 6/9] Use explicit BuildAction priority mapping --- src/ImageBuilder/Build/BuildPlan.cs | 22 ++++++++++++++++++---- src/ImageBuilder/Build/BuildPolicies.cs | 8 +++++--- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/ImageBuilder/Build/BuildPlan.cs b/src/ImageBuilder/Build/BuildPlan.cs index ff817248b..1fda4973e 100644 --- a/src/ImageBuilder/Build/BuildPlan.cs +++ b/src/ImageBuilder/Build/BuildPlan.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Collections.Generic; using Microsoft.DotNet.ImageBuilder.Models.Image; @@ -15,24 +16,37 @@ public enum BuildAction /// /// The published image is valid and this invocation does not need it locally. /// - NoAction = 0, + NoAction, /// /// Use the valid published image without running a Docker build. The image may need to be /// pulled, imported, or retagged for this invocation. /// - UsePublishedImage = 1, + UsePublishedImage, /// /// Use the valid published image and continue it through downstream processing because its /// published metadata, such as tags, must be updated. /// - PublishExistingImage = 2, + PublishExistingImage, /// /// Run a Docker build for the target. /// - BuildImage = 3 + BuildImage +} + +public static class BuildActionExtensions +{ + public static int GetPriority(this BuildAction action) => + action switch + { + BuildAction.NoAction => 0, + BuildAction.UsePublishedImage => 1, + BuildAction.PublishExistingImage => 2, + BuildAction.BuildImage => 3, + _ => throw new ArgumentOutOfRangeException(nameof(action)) + }; } /// diff --git a/src/ImageBuilder/Build/BuildPolicies.cs b/src/ImageBuilder/Build/BuildPolicies.cs index 893220890..db8faed19 100644 --- a/src/ImageBuilder/Build/BuildPolicies.cs +++ b/src/ImageBuilder/Build/BuildPolicies.cs @@ -45,7 +45,7 @@ Task EvaluateAsync( /// /// Applies every child policy and combines their results into one decision. The action with the -/// highest value wins. +/// highest priority wins. /// public sealed class CompositeBuildPolicy( BuildAction defaultAction, @@ -63,9 +63,11 @@ public async Task EvaluateAsync( BuildAction childAction = results .Select(result => result.Action) .DefaultIfEmpty(BuildAction.NoAction) - .Max(); + .MaxBy(action => action.GetPriority()); - BuildAction action = (BuildAction)Math.Max((int)childAction, (int)defaultAction); + BuildAction action = childAction.GetPriority() >= defaultAction.GetPriority() + ? childAction + : defaultAction; BuildReason[] reasons = results .SelectMany(result => result.Reasons) From 42a376d4308209916426294084f99e79f52025e2 Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Fri, 7 Aug 2026 11:11:32 -0700 Subject: [PATCH 7/9] Track only one build decision and log lower priority decisions --- .../Build/BuildPlannerTests.cs | 98 +++++++------- .../GenerateBuildMatrixCommandTests.cs | 12 +- src/ImageBuilder/Build/BuildGraph.cs | 6 +- src/ImageBuilder/Build/BuildPlan.cs | 5 +- src/ImageBuilder/Build/BuildPlanner.cs | 121 +++++++----------- src/ImageBuilder/Build/BuildPolicies.cs | 99 +++++++------- src/ImageBuilder/Commands/BuildCommand.cs | 19 ++- .../Commands/GenerateBuildMatrixCommand.cs | 8 +- .../Commands/GetStaleImagesCommand.cs | 8 +- 9 files changed, 193 insertions(+), 183 deletions(-) diff --git a/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs b/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs index 482486a05..6093354f1 100644 --- a/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs +++ b/src/ImageBuilder.Tests/Build/BuildPlannerTests.cs @@ -66,8 +66,10 @@ public async Task ChangedBaseImageExplainsFullDependencyChain() graph, imageInfo, new CompositeBuildPolicy( - BuildAction.NoAction, - new("All checks passed, so no work is required."), + new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason("All checks passed, so no work is required.")), + Mock.Of>(), new MissingPublishedImagePolicy(), CreateBaseImageRule(manifest, manifestService.Object), new TagSetChangedPolicy())); @@ -77,22 +79,21 @@ public async Task ChangedBaseImageExplainsFullDependencyChain() BuildPlanItem support = GetItem(plan, "support"); BuildPlanItem leaf = GetItem(plan, "leaf"); - root.Action.ShouldBe(BuildAction.BuildImage); - middle.Action.ShouldBe(BuildAction.BuildImage); - leaf.Action.ShouldBe(BuildAction.BuildImage); - support.Action.ShouldBe(BuildAction.UsePublishedImage); + root.Decision.Action.ShouldBe(BuildAction.BuildImage); + middle.Decision.Action.ShouldBe(BuildAction.BuildImage); + leaf.Decision.Action.ShouldBe(BuildAction.BuildImage); + support.Decision.Action.ShouldBe(BuildAction.UsePublishedImage); - BuildReason leafReason = leaf.Reasons.Last(reason => - reason.Message.StartsWith("Dependency", StringComparison.Ordinal)); - BuildReason middleReason = leafReason.Cause.ShouldNotBeNull(); + BuildReason leafReason = leaf.Decision.Reason; + leafReason.Message.ShouldStartWith("Dependency"); + BuildReason middleReason = leafReason.CausedBy.ShouldNotBeNull(); middleReason.Message.ShouldStartWith("Dependency"); - BuildReason rootReason = middleReason.Cause.ShouldNotBeNull(); + BuildReason rootReason = middleReason.CausedBy.ShouldNotBeNull(); rootReason.Message.ShouldContain("changed from 'sha256:old' to 'sha256:new'"); - BuildReason supportReason = support.Reasons.Single( - reason => reason.Message.StartsWith("The image is required", StringComparison.Ordinal)); + BuildReason supportReason = support.Decision.Reason; supportReason.Message.ShouldContain("leaf/Dockerfile"); - supportReason.Cause.ShouldBe(leafReason); + supportReason.CausedBy.ShouldBe(leafReason); } [TestMethod] @@ -120,15 +121,17 @@ public async Task ChangedTagSetsRequireReuse() graph, imageInfo, new CompositeBuildPolicy( - BuildAction.NoAction, - new("All checks passed, so no work is required."), + new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason("All checks passed, so no work is required.")), + Mock.Of>(), new MissingPublishedImagePolicy(), CreateBaseImageRule(manifest, manifestService.Object), new TagSetChangedPolicy())); BuildPlanItem item = plan.ShouldHaveSingleItem(); - item.Action.ShouldBe(BuildAction.PublishExistingImage); - BuildReason reason = item.Reasons.Last(); + item.Decision.Action.ShouldBe(BuildAction.PublishExistingImage); + BuildReason reason = item.Decision.Reason; reason.Message.ShouldContain("[old, removed]"); reason.Message.ShouldContain("[old, new]"); reason.Message.ShouldContain("[old-shared, new-shared]"); @@ -160,19 +163,21 @@ public async Task InvalidatedSharedBuildForcesEveryTargetToBuild() graph, imageInfo, new CompositeBuildPolicy( - BuildAction.NoAction, - new("All checks passed, so no work is required."), + new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason("All checks passed, so no work is required.")), + Mock.Of>(), new MissingPublishedImagePolicy(), CreateBaseImageRule(manifest, manifestService.Object), new TagSetChangedPolicy())); BuildPlanItem first = GetItem(plan, "first"); BuildPlanItem second = GetItem(plan, "second"); - first.Action.ShouldBe(BuildAction.BuildImage); - second.Action.ShouldBe(BuildAction.BuildImage); - BuildReason reason = second.Reasons.Last(); + first.Decision.Action.ShouldBe(BuildAction.BuildImage); + second.Decision.Action.ShouldBe(BuildAction.BuildImage); + BuildReason reason = second.Decision.Reason; reason.Message.ShouldContain("first"); - reason.Cause.ShouldNotBeNull().Message.ShouldContain("changed from"); + reason.CausedBy.ShouldNotBeNull().Message.ShouldContain("changed from"); } [TestMethod] @@ -203,15 +208,17 @@ public async Task SharedTagCreatesDependencyEdge() graph, imageInfo, new CompositeBuildPolicy( - BuildAction.NoAction, - new("All checks passed, so no work is required."), + new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason("All checks passed, so no work is required.")), + Mock.Of>(), new MissingPublishedImagePolicy(), CreateBaseImageRule(manifest, manifestService.Object), new TagSetChangedPolicy())); BuildPlanItem child = GetItem(plan, "child"); - child.Action.ShouldBe(BuildAction.BuildImage); - child.Reasons.Last().Message.ShouldStartWith("Dependency"); + child.Decision.Action.ShouldBe(BuildAction.BuildImage); + child.Decision.Reason.Message.ShouldStartWith("Dependency"); } [TestMethod] @@ -283,15 +290,17 @@ public async Task CachedSiblingIsNotIncludedWhenAnotherChildBuilds() graph, imageInfo, new CompositeBuildPolicy( - BuildAction.NoAction, - new("All checks passed, so no work is required."), + new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason("All checks passed, so no work is required.")), + Mock.Of>(), new MissingPublishedImagePolicy(), CreateBaseImageRule(manifest, manifestService.Object), new TagSetChangedPolicy())); - GetItem(plan, "parent").Action.ShouldBe(BuildAction.UsePublishedImage); - GetItem(plan, "first").Action.ShouldBe(BuildAction.BuildImage); - GetItem(plan, "second").Action.ShouldBe(BuildAction.NoAction); + GetItem(plan, "parent").Decision.Action.ShouldBe(BuildAction.UsePublishedImage); + GetItem(plan, "first").Decision.Action.ShouldBe(BuildAction.BuildImage); + GetItem(plan, "second").Decision.Action.ShouldBe(BuildAction.NoAction); } [TestMethod] @@ -308,13 +317,15 @@ public async Task CustomRuleMethodCanAddAPlanningDecision() graph, imageInfo: null, new CompositeBuildPolicy( - BuildAction.NoAction, - new("No package changed."), + new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason("No package changed.")), + Mock.Of>(), new PackageVersionChangedPolicy())); BuildPlanItem item = plan.ShouldHaveSingleItem(); - item.Action.ShouldBe(BuildAction.BuildImage); - item.Reasons.ShouldHaveSingleItem().Message.ShouldContain("openssl"); + item.Decision.Action.ShouldBe(BuildAction.BuildImage); + item.Decision.Reason.Message.ShouldContain("openssl"); } [TestMethod] @@ -329,18 +340,20 @@ public async Task CompositePolicyAppliesEveryChildAndChoosesStrongestAction() BuildGraph graph = BuildGraph.Create(manifest); List appliedPolicies = []; IBuildPolicy policy = new CompositeBuildPolicy( - BuildAction.NoAction, - new("No checks selected work."), + new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason("No checks selected work.")), + Mock.Of>(), new TestPolicy( appliedPolicies, "use", - new( + new BuildPolicyResult( BuildAction.UsePublishedImage, new BuildReason("Use the published image."))), new TestPolicy( appliedPolicies, "build", - new( + new BuildPolicyResult( BuildAction.BuildImage, new BuildReason("Build the image.")))); @@ -353,9 +366,8 @@ public async Task CompositePolicyAppliesEveryChildAndChoosesStrongestAction() appliedPolicies.ShouldBe(["use", "build"]); BuildPlanItem item = plan.ShouldHaveSingleItem(); - item.Action.ShouldBe(BuildAction.BuildImage); - item.Reasons.Select(reason => reason.Message).ShouldBe( - ["Use the published image.", "Build the image."]); + item.Decision.Action.ShouldBe(BuildAction.BuildImage); + item.Decision.Reason.Message.ShouldBe("Build the image."); } private static BuildPlanner CreatePlanner() => diff --git a/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs b/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs index 3fbb487a5..98d20d26f 100644 --- a/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs +++ b/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs @@ -300,13 +300,11 @@ public async Task FilterOutCachedImages( graph.Targets.Select(target => new BuildPlanItem( target, - actions.GetValueOrDefault( - target.Platform.DockerfilePathRelativeToManifest, - BuildAction.BuildImage), - Reasons: - [ - new("Test-selected action.") - ], + new BuildPolicyResult( + actions.GetValueOrDefault( + target.Platform.DockerfilePathRelativeToManifest, + BuildAction.BuildImage), + new BuildReason("Test-selected action.")), PublishedImage: null)) .ToArray())); diff --git a/src/ImageBuilder/Build/BuildGraph.cs b/src/ImageBuilder/Build/BuildGraph.cs index 71a5cce03..c33541b4d 100644 --- a/src/ImageBuilder/Build/BuildGraph.cs +++ b/src/ImageBuilder/Build/BuildGraph.cs @@ -18,7 +18,11 @@ public sealed record BuildTarget( RepoInfo Repo, ImageInfo Image, PlatformInfo Platform, - IReadOnlyDictionary FromImageOverrides); + IReadOnlyDictionary FromImageOverrides) +{ + public string DisplayName => + $"{Repo.Name} ({Platform.DockerfilePathRelativeToManifest})"; +} /// /// Dependency, shared-build, and published-image data for build targets. diff --git a/src/ImageBuilder/Build/BuildPlan.cs b/src/ImageBuilder/Build/BuildPlan.cs index 1fda4973e..e5ea9ef9f 100644 --- a/src/ImageBuilder/Build/BuildPlan.cs +++ b/src/ImageBuilder/Build/BuildPlan.cs @@ -54,7 +54,7 @@ public static int GetPriority(this BuildAction action) => /// public sealed record BuildReason( string Message, - BuildReason? Cause = null); + BuildReason? CausedBy = null); /// /// Published image-info associated with a build target. @@ -69,6 +69,5 @@ public sealed record PublishedImage( public sealed record BuildPlanItem( BuildTarget Target, - BuildAction Action, - IReadOnlyList Reasons, + BuildPolicyResult Decision, PublishedImage? PublishedImage); diff --git a/src/ImageBuilder/Build/BuildPlanner.cs b/src/ImageBuilder/Build/BuildPlanner.cs index 8a5fcafc9..fb5957f4e 100644 --- a/src/ImageBuilder/Build/BuildPlanner.cs +++ b/src/ImageBuilder/Build/BuildPlanner.cs @@ -62,6 +62,7 @@ await EvaluateAsync( BuildPlanItem[] plan = graph.Targets .Select(target => items[target]) .ToArray(); + LogPlan(plan); return plan; } @@ -130,21 +131,14 @@ private static async Task EvaluateAsync( bool hasPublishedImage = publishedImages.TryGetValue( target, out PublishedImage? publishedImage); - List reasons = []; - if (publishedImage is not null && publishedImage.Source != target) - { - reasons.Add(new( - $"Published image metadata is shared with '{GetName(publishedImage.Source)}'.")); - } - BuildPolicyResult result = await policy.EvaluateAsync( - new(graph, target, publishedImages), + BuildPolicyResult decision = await policy.EvaluateAsync( + new BuildPolicyContext(graph, target, publishedImages), cancellationToken); - reasons.AddRange(result.Reasons); + return CreateItem( target, - result.Action, - reasons, + decision, hasPublishedImage ? publishedImage : null); } @@ -175,7 +169,7 @@ void Visit(IReadOnlyList sharedBuild) if (!visiting.Add(key)) { throw new InvalidOperationException( - $"Build dependency cycle detected at '{GetName(key)}'."); + $"Build dependency cycle detected at '{key.DisplayName}'."); } foreach (BuildTarget parent in sharedBuild @@ -209,21 +203,27 @@ private static void PropagateBuildsFromParents( { foreach (BuildTarget target in sharedBuild) { - foreach (BuildTarget parent in graph.Parents[target] - .Where(parent => items[parent].Action == BuildAction.BuildImage)) + BuildPlanItem item = items[target]; + if (item.Decision.Action == BuildAction.BuildImage) { - BuildPlanItem item = items[target]; - BuildReason reason = new( - $"Dependency '{GetName(parent)}' must build.", - GetCause(items[parent])); - items[target] = item with - { - Action = BuildAction.BuildImage, - Reasons = item.Reasons.Contains(reason) - ? item.Reasons - : [..item.Reasons, reason] - }; + continue; + } + + BuildTarget? parent = graph.Parents[target] + .FirstOrDefault(parent => items[parent].Decision.Action == BuildAction.BuildImage); + if (parent is null) + { + continue; } + + items[target] = item with + { + Decision = new BuildPolicyResult( + BuildAction.BuildImage, + new BuildReason( + $"Dependency '{parent.DisplayName}' must build.", + items[parent].Decision.Reason)) + }; } } @@ -233,7 +233,7 @@ private static void UnifySharedBuildActions( { BuildPlanItem? invalidatedItem = sharedBuild .Select(target => items[target]) - .FirstOrDefault(item => item.Action == BuildAction.BuildImage); + .FirstOrDefault(item => item.Decision.Action == BuildAction.BuildImage); if (invalidatedItem is null) { return; @@ -242,21 +242,18 @@ private static void UnifySharedBuildActions( foreach (BuildTarget target in sharedBuild) { BuildPlanItem item = items[target]; - if (item.Action == BuildAction.BuildImage) + if (item.Decision.Action == BuildAction.BuildImage) { continue; } items[target] = item with { - Action = BuildAction.BuildImage, - Reasons = - [ - ..item.Reasons, - new( - $"Equivalent target '{GetName(invalidatedItem.Target)}' must build.", - GetCause(invalidatedItem)) - ] + Decision = new BuildPolicyResult( + BuildAction.BuildImage, + new BuildReason( + $"Equivalent target '{invalidatedItem.Target.DisplayName}' must build.", + invalidatedItem.Decision.Reason)) }; } } @@ -267,36 +264,27 @@ private static void UsePublishedParentsForBuilds( IReadOnlyDictionary publishedImages) { foreach (BuildPlanItem childItem in items.Values - .Where(item => item.Action == BuildAction.BuildImage) + .Where(item => item.Decision.Action == BuildAction.BuildImage) .ToArray()) { foreach (BuildTarget parent in graph.Parents[childItem.Target]) { BuildPlanItem parentItem = items[parent]; - BuildReason reason = new( - $"The image is required by '{GetName(childItem.Target)}'.", - GetCause(childItem)); + BuildPolicyResult decision = new BuildPolicyResult( + BuildAction.UsePublishedImage, + new BuildReason( + $"The image is required by '{childItem.Target.DisplayName}'.", + childItem.Decision.Reason)); - if (parentItem.Action == BuildAction.NoAction) + if (parentItem.Decision.Action == BuildAction.NoAction) { if (!publishedImages.ContainsKey(parent)) { throw new InvalidOperationException( - $"Required dependency '{GetName(parent)}' has no published image."); + $"Required dependency '{parent.DisplayName}' has no published image."); } - items[parent] = parentItem with - { - Action = BuildAction.UsePublishedImage, - Reasons = [reason, ..parentItem.Reasons] - }; - } - else if (!parentItem.Reasons.Contains(reason)) - { - items[parent] = parentItem with - { - Reasons = [reason, ..parentItem.Reasons] - }; + items[parent] = parentItem with { Decision = decision }; } } } @@ -304,19 +292,18 @@ private static void UsePublishedParentsForBuilds( private static BuildPlanItem CreateItem( BuildTarget target, - BuildAction action, - IEnumerable reasons, + BuildPolicyResult decision, PublishedImage? publishedImage) { - if ((action is BuildAction.UsePublishedImage or BuildAction.PublishExistingImage) && + if ((decision.Action is BuildAction.UsePublishedImage or BuildAction.PublishExistingImage) && publishedImage is null) { throw new InvalidOperationException( - $"Planning selected '{action}' for '{GetName(target)}' " + + $"Planning selected '{decision.Action}' for '{target.DisplayName}' " + "without a published image."); } - return new(target, action, reasons.ToArray(), publishedImage); + return new BuildPlanItem(target, decision, publishedImage); } private void LogPlan(IEnumerable plan) @@ -324,20 +311,10 @@ private void LogPlan(IEnumerable plan) foreach (BuildPlanItem item in plan) { _logger.LogInformation( - "Build plan for {DockerfilePath}: {Action}. {Reasons}", - GetName(item.Target), - item.Action, - string.Join(" ", item.Reasons.Select(FormatReason))); + "Build plan for {BuildTarget}: {Action}. {Reason}", + item.Target.DisplayName, + item.Decision.Action, + item.Decision.Reason); } } - - private static BuildReason GetCause(BuildPlanItem item) => item.Reasons.Last(); - - private static string FormatReason(BuildReason reason) => - reason.Cause is null - ? reason.Message - : $"{reason.Message} {FormatReason(reason.Cause)}"; - - private static string GetName(BuildTarget target) => - $"{target.Repo.Name} ({target.Platform.DockerfilePathRelativeToManifest})"; } diff --git a/src/ImageBuilder/Build/BuildPolicies.cs b/src/ImageBuilder/Build/BuildPolicies.cs index db8faed19..7e4b3aea1 100644 --- a/src/ImageBuilder/Build/BuildPolicies.cs +++ b/src/ImageBuilder/Build/BuildPolicies.cs @@ -19,19 +19,12 @@ public sealed record BuildPolicyContext( BuildTarget Target, IReadOnlyDictionary PublishedImages); +/// +/// Work selected by a build policy and the reason it was selected. +/// public sealed record BuildPolicyResult( BuildAction Action, - IReadOnlyList Reasons) -{ - public BuildPolicyResult( - BuildAction action, - BuildReason reason) - : this(action, [reason]) - { - } - - public static BuildPolicyResult None { get; } = new(BuildAction.NoAction, []); -} + BuildReason Reason); /// /// Evaluates one aspect of the work required for a build target. @@ -48,32 +41,34 @@ Task EvaluateAsync( /// highest priority wins. /// public sealed class CompositeBuildPolicy( - BuildAction defaultAction, - BuildReason defaultReason, - params IEnumerable policies) : IBuildPolicy + BuildPolicyResult defaultResult, + ILogger logger, + params IBuildPolicy[] policies) : IBuildPolicy { public async Task EvaluateAsync( BuildPolicyContext context, CancellationToken cancellationToken = default) { + BuildPolicyResult result = defaultResult; + foreach (IBuildPolicy policy in policies) + { + BuildPolicyResult policyResult = + await policy.EvaluateAsync(context, cancellationToken); + + logger.LogDebug( + "Build policy {BuildPolicy} for {BuildTarget} returned {Action}. {Reason}", + policy.GetType().Name, + context.Target.DisplayName, + policyResult.Action, + policyResult.Reason); + + if (policyResult.Action.GetPriority() > result.Action.GetPriority()) + { + result = policyResult; + } + } - BuildPolicyResult[] results = await Task.WhenAll( - policies.Select(policy => policy.EvaluateAsync(context, cancellationToken))); - - BuildAction childAction = results - .Select(result => result.Action) - .DefaultIfEmpty(BuildAction.NoAction) - .MaxBy(action => action.GetPriority()); - - BuildAction action = childAction.GetPriority() >= defaultAction.GetPriority() - ? childAction - : defaultAction; - - BuildReason[] reasons = results - .SelectMany(result => result.Reasons) - .ToArray(); - - return new BuildPolicyResult(action, childAction == BuildAction.NoAction ? [..reasons, defaultReason] : reasons); + return result; } } @@ -99,10 +94,12 @@ public Task EvaluateAsync( { cancellationToken.ThrowIfCancellationRequested(); BuildPolicyResult result = !context.PublishedImages.ContainsKey(context.Target) - ? new( + ? new BuildPolicyResult( BuildAction.BuildImage, new BuildReason("No published image metadata exists.")) - : BuildPolicyResult.None; + : new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason("Published image metadata exists.")); return Task.FromResult(result); } } @@ -116,7 +113,11 @@ public Task EvaluateAsync( cancellationToken.ThrowIfCancellationRequested(); if (!context.PublishedImages.TryGetValue(context.Target, out var publishedImage)) { - return Task.FromResult(BuildPolicyResult.None); + return Task.FromResult( + new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason( + "Published image metadata is unavailable, so tags cannot be compared."))); } string[] expectedPlatformTags = context.Target.Platform.Tags @@ -138,14 +139,16 @@ public Task EvaluateAsync( !expectedSharedTags.AreEquivalent(publishedSharedTags); BuildPolicyResult result = tagsChanged - ? new( + ? new BuildPolicyResult( BuildAction.PublishExistingImage, new BuildReason( $"Platform tags changed from [{string.Join(", ", publishedPlatformTags)}] " + $"to [{string.Join(", ", expectedPlatformTags)}]; shared tags changed from " + $"[{string.Join(", ", publishedSharedTags)}] to " + $"[{string.Join(", ", expectedSharedTags)}].")) - : BuildPolicyResult.None; + : new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason("Configured tags are unchanged.")); return Task.FromResult(result); } } @@ -164,7 +167,11 @@ public Task EvaluateAsync( cancellationToken.ThrowIfCancellationRequested(); if (!context.PublishedImages.TryGetValue(context.Target, out var publishedImage)) { - return Task.FromResult(BuildPolicyResult.None); + return Task.FromResult( + new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason( + "Published image metadata is unavailable, so the Dockerfile cannot be compared."))); } string currentCommitUrl = _gitService.GetDockerfileCommitUrl( @@ -174,10 +181,10 @@ public Task EvaluateAsync( currentCommitUrl, StringComparison.OrdinalIgnoreCase); BuildPolicyResult result = matches - ? new( + ? new BuildPolicyResult( BuildAction.NoAction, new BuildReason($"Dockerfile is unchanged at '{currentCommitUrl}'.")) - : new( + : new BuildPolicyResult( BuildAction.BuildImage, new BuildReason( $"Dockerfile changed from '{publishedImage.Image.CommitUrl}' " + @@ -224,13 +231,16 @@ public async Task EvaluateAsync( cancellationToken.ThrowIfCancellationRequested(); if (!context.PublishedImages.TryGetValue(context.Target, out var publishedImage)) { - return BuildPolicyResult.None; + return new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason( + "Published image metadata is unavailable, so the base image cannot be compared.")); } string? fromImage = context.Target.Platform.FinalStageFromImage; if (fromImage is null) { - return new( + return new BuildPolicyResult( BuildAction.NoAction, new BuildReason("The final stage has no base image.")); } @@ -245,16 +255,17 @@ public async Task EvaluateAsync( currentValue, StringComparison.OrdinalIgnoreCase) == true; - return matches - ? new( + BuildPolicyResult result = matches + ? new BuildPolicyResult( BuildAction.NoAction, new BuildReason( $"Base image '{publicImage}' is unchanged at '{currentValue}'.")) - : new( + : new BuildPolicyResult( BuildAction.BuildImage, new BuildReason( $"Base image '{publicImage}' changed from " + $"'{Display(previousValue)}' to '{Display(currentValue)}'.")); + return result; } private static string? GetInternalBaseImageDigest( diff --git a/src/ImageBuilder/Commands/BuildCommand.cs b/src/ImageBuilder/Commands/BuildCommand.cs index 4bed7ed46..921107004 100644 --- a/src/ImageBuilder/Commands/BuildCommand.cs +++ b/src/ImageBuilder/Commands/BuildCommand.cs @@ -322,8 +322,11 @@ private Task CreateBuildPlanAsync(BuildGraph graph, ImageArtifa IBuildPolicy policy = Options.NoCache ? new AlwaysBuildPolicy() : new CompositeBuildPolicy( - defaultAction: BuildAction.UsePublishedImage, - defaultReason: new BuildReason("All checks passed, so this invocation will use the published image."), + defaultResult: new BuildPolicyResult( + BuildAction.UsePublishedImage, + new BuildReason( + "All checks passed, so this invocation will use the published image.")), + logger: _logger, policies: [ // Rebuild when no published image metadata exists. @@ -351,10 +354,12 @@ private async Task BuildImagesAsync(IEnumerable plan) { _logger.LogInformation("BUILDING IMAGES"); - BuildPlanItem[] executableItems = plan.Where(item => item.Action != BuildAction.NoAction) .ToArray(); + BuildPlanItem[] executableItems = plan + .Where(item => item.Decision.Action != BuildAction.NoAction) + .ToArray(); _hasPublishedImagesToUse = executableItems.Any( - item => item.Action + item => item.Decision.Action is BuildAction.UsePublishedImage or BuildAction.PublishExistingImage); @@ -389,18 +394,18 @@ is BuildAction.UsePublishedImage PlatformData platformData = CreatePlatformData(image, platform); imageData.Platforms.Add(platformData); - if (plannedImage.Action is BuildAction.UsePublishedImage or BuildAction.PublishExistingImage) + if (plannedImage.Decision.Action is BuildAction.UsePublishedImage or BuildAction.PublishExistingImage) { PublishedImage publishedImage = plannedImage.PublishedImage ?? throw new InvalidOperationException( $"Build plan did not provide reusable metadata for '{platform.DockerfilePath}'."); CopyPlatformDataFromCachedPlatform(platformData, publishedImage.Image); - platformData.IsUnchanged = plannedImage.Action == BuildAction.UsePublishedImage; + platformData.IsUnchanged = plannedImage.Decision.Action == BuildAction.UsePublishedImage; await UsePublishedImageAsync(repoInfo, allTagInfos, publishedImage.Image.Digest); } - else if (plannedImage.Action == BuildAction.BuildImage) + else if (plannedImage.Decision.Action == BuildAction.BuildImage) { _processedTags.AddRange(allTagInfos); diff --git a/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs b/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs index de82fce00..c0a8150f0 100644 --- a/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs +++ b/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs @@ -442,8 +442,10 @@ private async Task> GetPlatformsToBuildAsync() // ...and then check which images actually need to be built. IBuildPolicy policy = Options.TrimCachedImages ? new CompositeBuildPolicy( - defaultAction: BuildAction.NoAction, - defaultReason: new BuildReason("All checks passed, so no work is required."), + defaultResult: new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason("All checks passed, so no work is required.")), + logger: _logger, policies: [ // Rebuild when no published image metadata exists. @@ -468,7 +470,7 @@ private async Task> GetPlatformsToBuildAsync() BuildPlanItem[] plan = await _buildPlanner.CreatePlanAsync(graph, imageInfo, policy); IEnumerable plannedPlatforms = plan - .Where(item => item.Action != BuildAction.NoAction) + .Where(item => item.Decision.Action != BuildAction.NoAction) .Select(item => item.Target.Platform); return Options.TrimCachedImages diff --git a/src/ImageBuilder/Commands/GetStaleImagesCommand.cs b/src/ImageBuilder/Commands/GetStaleImagesCommand.cs index 745238a2b..31c839149 100644 --- a/src/ImageBuilder/Commands/GetStaleImagesCommand.cs +++ b/src/ImageBuilder/Commands/GetStaleImagesCommand.cs @@ -103,8 +103,10 @@ private async Task> GetPathsToRebuildAsync(Models.Subscripti BuildGraph graph = BuildGraph.CreateFiltered(manifest); IBuildPolicy policy = new CompositeBuildPolicy( - defaultAction: BuildAction.NoAction, - defaultReason: new BuildReason("All checks passed, so no work is required."), + defaultResult: new BuildPolicyResult( + BuildAction.NoAction, + new BuildReason("All checks passed, so no work is required.")), + logger: _logger, policies: [ // Rebuild when no published image metadata exists. @@ -120,7 +122,7 @@ private async Task> GetPathsToRebuildAsync(Models.Subscripti BuildPlanItem[] plan = await _buildPlanner.CreatePlanAsync(graph, imageArtifactDetails, policy); return plan - .Where(item => item.Action != BuildAction.NoAction) + .Where(item => item.Decision.Action != BuildAction.NoAction) .Select(item => item.Target.Platform.Model.Dockerfile) .Distinct(); } From 5d31ab9ccca72a9f729be10389bdad8c7a3d482f Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Mon, 10 Aug 2026 09:07:28 -0700 Subject: [PATCH 8/9] Clean up BuildPlanner --- src/ImageBuilder/Build/BuildPlanner.cs | 197 ++++++++++--------------- 1 file changed, 78 insertions(+), 119 deletions(-) diff --git a/src/ImageBuilder/Build/BuildPlanner.cs b/src/ImageBuilder/Build/BuildPlanner.cs index fb5957f4e..96249dad9 100644 --- a/src/ImageBuilder/Build/BuildPlanner.cs +++ b/src/ImageBuilder/Build/BuildPlanner.cs @@ -17,101 +17,90 @@ namespace Microsoft.DotNet.ImageBuilder.Build; /// public class BuildPlanner(ILogger logger) { - private readonly ILogger _logger = - logger ?? throw new ArgumentNullException(nameof(logger)); - public virtual async Task CreatePlanAsync( BuildGraph graph, ImageArtifactDetails? imageInfo, IBuildPolicy policy, CancellationToken cancellationToken = default) { - ArgumentNullException.ThrowIfNull(graph); - ArgumentNullException.ThrowIfNull(policy); - - Dictionary publishedImages = - CreatePublishedImageIndex(graph, imageInfo); - - Dictionary items = []; + Dictionary publishedImages = CreatePublishedImageIndex(graph, imageInfo); + Dictionary planItems = []; // Shared builds act as one node. Evaluate and apply each node from roots to leaves so // every parent decision is final before its children are considered. - foreach (IReadOnlyList sharedBuild in - GetSharedBuildsInDependencyOrder(graph)) + foreach (var sharedBuildTargets in GetSharedBuildsInDependencyOrder(graph)) { - foreach (BuildTarget target in sharedBuild) + foreach (BuildTarget target in sharedBuildTargets) { - items.Add( - target, - await EvaluateAsync( - graph, - target, - publishedImages, - policy, - cancellationToken)); + var context = new BuildPolicyContext(graph, target, publishedImages); + BuildPolicyResult decision = await policy.EvaluateAsync(context, cancellationToken); + + publishedImages.TryGetValue(target, out PublishedImage? publishedImage); + planItems.Add(target, CreateItem(target, decision, publishedImage)); } - PropagateBuildsFromParents(graph, sharedBuild, items); - UnifySharedBuildActions(sharedBuild, items); + PropagateBuildsFromParents(graph, sharedBuildTargets, planItems); + UnifySharedBuildActions(sharedBuildTargets, planItems); } // Built images need their direct internal parents available locally. An unchanged parent // can use its published image; it does not need its own parents because it is not rebuilt. - UsePublishedParentsForBuilds(graph, items, publishedImages); - - BuildPlanItem[] plan = graph.Targets - .Select(target => items[target]) - .ToArray(); + UsePublishedParentsForBuilds(graph, planItems, publishedImages); + BuildPlanItem[] plan = graph.Targets.Select(target => planItems[target]).ToArray(); LogPlan(plan); return plan; } + /// + /// Finds previously published image data for each build target. + /// private static Dictionary CreatePublishedImageIndex( BuildGraph graph, ImageArtifactDetails? imageInfo) { - Dictionary targetsByPlatform = graph.Targets.ToDictionary( - target => target.Platform); - Dictionary publishedImages = imageInfo?.Repos + Dictionary targetsByPlatform = + graph.Targets.ToDictionary(target => target.Platform); + + Dictionary publishedImages = + imageInfo?.Repos + // Collect (image, platform) pairs for each platform .SelectMany(repo => repo.Images) - .SelectMany(image => image.Platforms.Select(platform => - ( - Platform: platform, - SharedTags: image.Manifest?.SharedTags?.ToArray() ?? []))) + .SelectMany( + image => image.Platforms.Select( + platform => (Platform: platform, SharedTags: image.Manifest?.SharedTags?.ToArray() ?? []) + ) + ) .Where(item => - item.Platform.PlatformInfo is not null && - targetsByPlatform.ContainsKey(item.Platform.PlatformInfo)) - .GroupBy(item => item.Platform.PlatformInfo!) + // PlatformInfo is only set when a published platform matches one in the current manifest. + item.Platform.PlatformInfo is not null + // Only select platforms that are part of the current build graph. + && targetsByPlatform.ContainsKey(item.Platform.PlatformInfo) + ) .ToDictionary( - group => targetsByPlatform[group.Key], - group => - { - var item = group.First(); - BuildTarget target = targetsByPlatform[group.Key]; - return new PublishedImage( - target, - item.Platform, - item.SharedTags); - }) + // SAFETY: PlatformInfo is checked in the Where clause above. + item => targetsByPlatform[item.Platform.PlatformInfo!], + item => new PublishedImage( + targetsByPlatform[item.Platform.PlatformInfo!], + item.Platform, + item.SharedTags) + ) ?? []; // Equivalent targets can reuse one another's published image when only one target has // a direct image-info entry. - foreach (IReadOnlyList sharedBuild in - graph.SharedBuildTargets.Values.Distinct()) + foreach (IReadOnlyList sharedBuild in graph.SharedBuildTargets.Values.Distinct()) { BuildTarget? source = sharedBuild.FirstOrDefault(publishedImages.ContainsKey); + if (source is null) - { continue; - } foreach (BuildTarget target in sharedBuild) { publishedImages.TryAdd( - target, - new PublishedImage( + key: target, + value: new PublishedImage( source, publishedImages[source].Image, publishedImages[source].SharedTags)); @@ -121,39 +110,17 @@ item.Platform.PlatformInfo is not null && return publishedImages; } - private static async Task EvaluateAsync( - BuildGraph graph, - BuildTarget target, - IReadOnlyDictionary publishedImages, - IBuildPolicy policy, - CancellationToken cancellationToken) - { - bool hasPublishedImage = publishedImages.TryGetValue( - target, - out PublishedImage? publishedImage); - - BuildPolicyResult decision = await policy.EvaluateAsync( - new BuildPolicyContext(graph, target, publishedImages), - cancellationToken); - - return CreateItem( - target, - decision, - hasPublishedImage ? publishedImage : null); - } - - private static List> - GetSharedBuildsInDependencyOrder(BuildGraph graph) + private static List> GetSharedBuildsInDependencyOrder(BuildGraph graph) { - IReadOnlyList[] sharedBuilds = graph.SharedBuildTargets - .Values + IReadOnlyList[] sharedBuilds = graph.SharedBuildTargets.Values .DistinctBy(targets => targets[0]) .ToArray(); - Dictionary> sharedBuildByTarget = + + Dictionary> sharedBuildsByTarget = sharedBuilds - .SelectMany(sharedBuild => sharedBuild.Select( - target => (Target: target, SharedBuild: sharedBuild))) + .SelectMany(sharedBuild => sharedBuild.Select(target => (Target: target, SharedBuild: sharedBuild))) .ToDictionary(item => item.Target, item => item.SharedBuild); + List> ordered = []; HashSet visiting = []; HashSet visited = []; @@ -161,26 +128,20 @@ private static List> void Visit(IReadOnlyList sharedBuild) { BuildTarget key = sharedBuild[0]; + if (visited.Contains(key)) - { return; - } if (!visiting.Add(key)) - { - throw new InvalidOperationException( - $"Build dependency cycle detected at '{key.DisplayName}'."); - } + throw new InvalidOperationException($"Build dependency cycle detected at '{key.DisplayName}'."); + + var parents = sharedBuild.SelectMany(target => graph.Parents[target]).Distinct(); - foreach (BuildTarget parent in sharedBuild - .SelectMany(target => graph.Parents[target]) - .Distinct()) + foreach (BuildTarget parent in parents) { - IReadOnlyList parentBuild = sharedBuildByTarget[parent]; + IReadOnlyList parentBuild = sharedBuildsByTarget[parent]; if (parentBuild[0] != key) - { Visit(parentBuild); - } } visiting.Remove(key); @@ -199,22 +160,20 @@ void Visit(IReadOnlyList sharedBuild) private static void PropagateBuildsFromParents( BuildGraph graph, IEnumerable sharedBuild, - IDictionary items) + Dictionary items) { foreach (BuildTarget target in sharedBuild) { BuildPlanItem item = items[target]; + if (item.Decision.Action == BuildAction.BuildImage) - { continue; - } BuildTarget? parent = graph.Parents[target] .FirstOrDefault(parent => items[parent].Decision.Action == BuildAction.BuildImage); + if (parent is null) - { continue; - } items[target] = item with { @@ -229,23 +188,21 @@ private static void PropagateBuildsFromParents( private static void UnifySharedBuildActions( IEnumerable sharedBuild, - IDictionary items) + Dictionary items) { BuildPlanItem? invalidatedItem = sharedBuild .Select(target => items[target]) .FirstOrDefault(item => item.Decision.Action == BuildAction.BuildImage); + if (invalidatedItem is null) - { return; - } foreach (BuildTarget target in sharedBuild) { BuildPlanItem item = items[target]; + if (item.Decision.Action == BuildAction.BuildImage) - { continue; - } items[target] = item with { @@ -260,31 +217,33 @@ private static void UnifySharedBuildActions( private static void UsePublishedParentsForBuilds( BuildGraph graph, - IDictionary items, - IReadOnlyDictionary publishedImages) + Dictionary items, + Dictionary publishedImages) { - foreach (BuildPlanItem childItem in items.Values + var childrenToBuild = items.Values .Where(item => item.Decision.Action == BuildAction.BuildImage) - .ToArray()) + .ToArray(); + + foreach (BuildPlanItem childItem in childrenToBuild) { foreach (BuildTarget parent in graph.Parents[childItem.Target]) { BuildPlanItem parentItem = items[parent]; - BuildPolicyResult decision = new BuildPolicyResult( - BuildAction.UsePublishedImage, - new BuildReason( - $"The image is required by '{childItem.Target.DisplayName}'.", - childItem.Decision.Reason)); if (parentItem.Decision.Action == BuildAction.NoAction) { if (!publishedImages.ContainsKey(parent)) - { throw new InvalidOperationException( $"Required dependency '{parent.DisplayName}' has no published image."); - } - items[parent] = parentItem with { Decision = decision }; + items[parent] = parentItem with + { + Decision = new BuildPolicyResult( + Action: BuildAction.UsePublishedImage, + Reason: new BuildReason( + $"The image is required by '{childItem.Target.DisplayName}'.", + childItem.Decision.Reason)) + }; } } } @@ -295,8 +254,8 @@ private static BuildPlanItem CreateItem( BuildPolicyResult decision, PublishedImage? publishedImage) { - if ((decision.Action is BuildAction.UsePublishedImage or BuildAction.PublishExistingImage) && - publishedImage is null) + if ((decision.Action is BuildAction.UsePublishedImage or BuildAction.PublishExistingImage) + && publishedImage is null) { throw new InvalidOperationException( $"Planning selected '{decision.Action}' for '{target.DisplayName}' " + @@ -310,7 +269,7 @@ private void LogPlan(IEnumerable plan) { foreach (BuildPlanItem item in plan) { - _logger.LogInformation( + logger.LogInformation( "Build plan for {BuildTarget}: {Action}. {Reason}", item.Target.DisplayName, item.Decision.Action, From 29742fb5142b44bcb855e1331cb155745373f169 Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Mon, 10 Aug 2026 13:04:43 -0700 Subject: [PATCH 9/9] Add common build policy helper --- src/ImageBuilder/Build/BuildPolicies.cs | 26 +++++++++++++++++ src/ImageBuilder/Commands/BuildCommand.cs | 27 +++++------------ .../Commands/GenerateBuildMatrixCommand.cs | 29 +++++-------------- .../Commands/GetStaleImagesCommand.cs | 2 ++ 4 files changed, 43 insertions(+), 41 deletions(-) diff --git a/src/ImageBuilder/Build/BuildPolicies.cs b/src/ImageBuilder/Build/BuildPolicies.cs index 7e4b3aea1..7e11fab4c 100644 --- a/src/ImageBuilder/Build/BuildPolicies.cs +++ b/src/ImageBuilder/Build/BuildPolicies.cs @@ -36,6 +36,32 @@ Task EvaluateAsync( CancellationToken cancellationToken = default); } +public static class CommonBuildPolicies +{ + public static CompositeBuildPolicy CreateForCachedImages( + BuildPolicyResult defaultResult, + ILogger logger, + BaseImageChangedPolicy baseImagePolicy, + IGitService gitService, + string sourceRepoUrl) + { + return new CompositeBuildPolicy( + defaultResult, + logger, + // Rebuild when no published image metadata exists. + new MissingPublishedImagePolicy(), + + // Rebuild when the base image digest has changed. + baseImagePolicy, + + // Rebuild when the Dockerfile has changed. + new DockerfileChangedPolicy(gitService, sourceRepoUrl), + + // Republish the existing image when its configured tags have changed. + new TagSetChangedPolicy()); + } +} + /// /// Applies every child policy and combines their results into one decision. The action with the /// highest priority wins. diff --git a/src/ImageBuilder/Commands/BuildCommand.cs b/src/ImageBuilder/Commands/BuildCommand.cs index 921107004..c418709cc 100644 --- a/src/ImageBuilder/Commands/BuildCommand.cs +++ b/src/ImageBuilder/Commands/BuildCommand.cs @@ -321,31 +321,18 @@ private Task CreateBuildPlanAsync(BuildGraph graph, ImageArtifa { IBuildPolicy policy = Options.NoCache ? new AlwaysBuildPolicy() - : new CompositeBuildPolicy( + : CommonBuildPolicies.CreateForCachedImages( defaultResult: new BuildPolicyResult( BuildAction.UsePublishedImage, new BuildReason( "All checks passed, so this invocation will use the published image.")), logger: _logger, - policies: - [ - // Rebuild when no published image metadata exists. - new MissingPublishedImagePolicy(), - - // Rebuild when the locally available base image digest has changed. - BaseImageChangedPolicy.FromLocalImages( - _imageDigestCache, - _imageNameResolver.Value, - Options.IsDryRun), - - // Rebuild when the Dockerfile has changed. - new DockerfileChangedPolicy( - _gitService, - sourceRepoUrl: Options.SourceRepoUrl ?? string.Empty), - - // Republish the existing image when its configured tags have changed. - new TagSetChangedPolicy() - ]); + baseImagePolicy: BaseImageChangedPolicy.FromLocalImages( + _imageDigestCache, + _imageNameResolver.Value, + Options.IsDryRun), + gitService: _gitService, + sourceRepoUrl: Options.SourceRepoUrl ?? string.Empty); return _buildPlanner.CreatePlanAsync(graph, publishedImages, policy); } diff --git a/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs b/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs index c0a8150f0..de5c72f30 100644 --- a/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs +++ b/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs @@ -441,31 +441,18 @@ private async Task> GetPlatformsToBuildAsync() // ...and then check which images actually need to be built. IBuildPolicy policy = Options.TrimCachedImages - ? new CompositeBuildPolicy( + ? CommonBuildPolicies.CreateForCachedImages( defaultResult: new BuildPolicyResult( BuildAction.NoAction, new BuildReason("All checks passed, so no work is required.")), logger: _logger, - policies: - [ - // Rebuild when no published image metadata exists. - new MissingPublishedImagePolicy(), - - // Rebuild when the registry digest for the base image has changed. - BaseImageChangedPolicy.FromRegistry( - _imageDigestCache, - _imageNameResolver.Value, - Options.IsDryRun), - - // Rebuild when the Dockerfile has changed. - new DockerfileChangedPolicy( - _gitService, - sourceRepoUrl: Options.SourceRepoUrl ?? string.Empty), - - // Republish the existing image when its configured tags have changed. - new TagSetChangedPolicy() - ]) - : new AlwaysBuildPolicy("The image was selected for a build."); + baseImagePolicy: BaseImageChangedPolicy.FromRegistry( + _imageDigestCache, + _imageNameResolver.Value, + Options.IsDryRun), + gitService: _gitService, + sourceRepoUrl: Options.SourceRepoUrl ?? string.Empty) + : new AlwaysBuildPolicy(); BuildPlanItem[] plan = await _buildPlanner.CreatePlanAsync(graph, imageInfo, policy); diff --git a/src/ImageBuilder/Commands/GetStaleImagesCommand.cs b/src/ImageBuilder/Commands/GetStaleImagesCommand.cs index 31c839149..fded3b761 100644 --- a/src/ImageBuilder/Commands/GetStaleImagesCommand.cs +++ b/src/ImageBuilder/Commands/GetStaleImagesCommand.cs @@ -102,6 +102,8 @@ private async Task> GetPathsToRebuildAsync(Models.Subscripti BuildGraph graph = BuildGraph.CreateFiltered(manifest); + // This command only reports images made stale by missing metadata or base image updates. + // Dockerfile and tag changes are handled by normal build planning. IBuildPolicy policy = new CompositeBuildPolicy( defaultResult: new BuildPolicyResult( BuildAction.NoAction,