diff --git a/modules/mesh-delivery-builder/internal/build/claim.go b/modules/mesh-delivery-builder/internal/build/claim.go index d598bc7..c99779f 100644 --- a/modules/mesh-delivery-builder/internal/build/claim.go +++ b/modules/mesh-delivery-builder/internal/build/claim.go @@ -108,7 +108,8 @@ func Unreferenced(b delivery.BuilderBuild) bool { // when a second node qualifies now (ClaimOptions.MoreNodes): that is taken as it stands, with the done value, and its // builder writes the new revision when the second build ends (ADR 0478 clause 4); // - a failed key is taken again as a new revision of the same key (ADR 0429 clause 18), except one failed for a kind no -// builder builds yet (RefusedPrefix, ADR 0478 clause 2) or for digests that differ (buildrun.DigestsDifferPrefix, clause 4), +// builder builds, for an upstream pinned by no digest or a package version taken (delivery.FailedUntilChanged: ADR 0478 +// clause 2, ADR 0498 clauses 4 and 5) or for digests that differ (buildrun.DigestsDifferPrefix, clause 4), // which wait for another fingerprint, and one failed for want of an image builder (NoImageBuilderPrefix), which waits for // another fingerprint or a node that runs the image build module (ClaimOptions.ImageBuilder); // - a key left queued or building by this builder is taken as it stands (ADR 0478 clause 5: a stopped builder takes its @@ -142,7 +143,7 @@ func (c Claimer) Claim(ctx context.Context, module, fingerprint, manifest string return &Claim{Key: key, Rev: rev, Build: cur}, nil } return nil, nil - case cur.State == delivery.BuildFailed && (strings.HasPrefix(cur.Error, RefusedPrefix) || strings.HasPrefix(cur.Error, buildrun.DigestsDifferPrefix)): + case cur.State == delivery.BuildFailed && (delivery.FailedUntilChanged(cur.Error) || strings.HasPrefix(cur.Error, buildrun.DigestsDifferPrefix)): return nil, nil case cur.State == delivery.BuildFailed && strings.HasPrefix(cur.Error, NoImageBuilderPrefix) && (opt.ImageBuilder == nil || !opt.ImageBuilder()): return nil, nil diff --git a/modules/mesh-delivery-builder/internal/build/claim_test.go b/modules/mesh-delivery-builder/internal/build/claim_test.go index fdf6d93..ed359a3 100644 --- a/modules/mesh-delivery-builder/internal/build/claim_test.go +++ b/modules/mesh-delivery-builder/internal/build/claim_test.go @@ -148,11 +148,19 @@ func TestAFailedKeyIsTakenAgainAsANewRevisionExceptARefusedKind(t *testing.T) { } st = newMem() - key = seed(t, st, delivery.BuilderBuild{State: delivery.BuildFailed, By: "novox.a", Error: RefusedPrefix + " image (artifact x): build-agent builds it"}) + key = seed(t, st, delivery.BuilderBuild{State: delivery.BuildFailed, By: "novox.a", Error: delivery.RefusedKind + "package of language \"python\" (artifact x)"}) writes := st.writes if got, err := (Claimer{Store: st, By: "novox.a"}).Claim(context.Background(), "m", fp1, "", ClaimOptions{}); err != nil || got != nil || st.writes != writes { t.Errorf("a refused kind: claim = %v, %v, writes %d (was %d), want none and no write", got, err, st.writes, writes) } + for _, e := range []string{delivery.NotPinned + "up: nats:2 names no digest", delivery.VersionTaken + "@novox/mesh-sdk@0.1.15"} { // B3, B5: ADR 0498 clauses 4, 5 + st = newMem() + seed(t, st, delivery.BuilderBuild{State: delivery.BuildFailed, By: "novox.a", Error: e}) + writes = st.writes + if got, err := (Claimer{Store: st, By: "novox.a"}).Claim(context.Background(), "m", fp1, "", ClaimOptions{}); err != nil || got != nil || st.writes != writes { + t.Errorf("%s: claim = %v, %v, writes %d (was %d), want none until the fingerprint changes", e, got, err, st.writes, writes) + } + } } func TestAKeyFailedForNoImageBuilderIsTakenAgainOnlyWhenANodeHasTheModule(t *testing.T) { // 1705b diff --git a/modules/mesh-delivery-builder/internal/build/plan.go b/modules/mesh-delivery-builder/internal/build/plan.go index dc1f6b7..33d48e6 100644 --- a/modules/mesh-delivery-builder/internal/build/plan.go +++ b/modules/mesh-delivery-builder/internal/build/plan.go @@ -5,16 +5,14 @@ package build import ( "fmt" - "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" "sort" + "strings" "git.novox.be/novox/mesh-sdk/go/catalogue" + "git.novox.be/novox/mesh-sdk/go/delivery" + "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" ) -// RefusedPrefix opens the error of a build key that is failed for a kind no builder builds yet (ADR 0478 clause 2): such -// a key is not built again as a new revision. -const RefusedPrefix = "refused kind:" - // ImageBuildModule is the build module that builds an image artifact (novox/hq ADR 0487): buildah, rootless, on the nodes that // run it. It has no toolchain language; the builder finds its nodes from the node-engine's have. const ImageBuildModule = "mesh-build-image" @@ -31,6 +29,9 @@ type Task struct { Kind string Module string Language string + // From is, for an upstream artifact, the reference its manifest pins, which the image build module copies (ADR 0498 + // clause 4). + From string } // Plan is what a manifest asks the builder to build, by artifact name. @@ -54,25 +55,34 @@ func (p Plan) Modules() []string { // (the indexer gives an archive no language, so a fingerprint names no toolchain for it; ADR 0478 clause 1). func (p Plan) NeedsToolchain(module string) bool { for _, t := range p.Tasks { - if t.Module == module && t.Kind != catalogue.ArtifactArchive && t.Kind != catalogue.ArtifactImage { + if t.Module == module && t.Kind != catalogue.ArtifactArchive && t.Kind != catalogue.ArtifactImage && t.Kind != catalogue.ArtifactUpstream { return true } } return false } -// Refused is the error of a plan the builder does not build: an artifact of a kind (or of a language) it has no build -// module for. +// Refused is the error of a plan the builder does not build: an artifact of a kind (or of a language) no build module +// builds. It opens delivery.RefusedKind, so its key is not built again until the fingerprint changes. type Refused struct{ Artifact, What string } func (r *Refused) Error() string { - return fmt.Sprintf("%s %s (artifact %s): the builder builds Go and TypeScript bundles and archives; build-agent builds it", RefusedPrefix, r.What, r.Artifact) + return fmt.Sprintf("%s%s (artifact %s): no build module builds it; the builder builds Go and TypeScript bundles, archives, images, upstream copies and TypeScript packages", delivery.RefusedKind, r.What, r.Artifact) } -// PlanOf is the plan of a manifest (ADR 0478 clauses 1 and 2): a Go bundle and an archive are built by the Go build -// module, an image by the image build module (ADR 0487), a TypeScript bundle by the Node one, each in the order of the artifacts' names. A manifest with an -// upstream or package artifact, or a bundle of another language, is refused as a whole, naming the first such artifact -// by name. A manifest with no artifact has an empty plan. +// NotPinned is the error of a plan with an upstream artifact whose reference names no digest (ADR 0498 clause 4): it opens +// delivery.NotPinned, so its key is not built again until the fingerprint changes. +type NotPinned struct{ Artifact, From string } + +func (n *NotPinned) Error() string { + return fmt.Sprintf("%s%s: %q names no digest (@sha256:), so what would be copied is whatever its tag means today", delivery.NotPinned, n.Artifact, n.From) +} + +// PlanOf is the plan of a manifest (ADR 0478 clauses 1 and 2, ADR 0498 clauses 4 and 5): a Go bundle and an archive are +// built by the Go build module, an image by the image build module (ADR 0487), which also copies an upstream image pinned +// by its digest, and a TypeScript bundle or package by the Node one, each in the order of the artifacts' names. A manifest +// with an upstream artifact pinned by no digest, or a bundle or a package of another language, or a kind no build module +// builds, is refused as a whole, naming the first such artifact by name. A manifest with no artifact has an empty plan. func PlanOf(m catalogue.Manifest) (Plan, error) { if m.Build == nil { return Plan{}, nil @@ -90,8 +100,14 @@ func PlanOf(m catalogue.Manifest) (Plan, error) { p.Tasks = append(p.Tasks, Task{Artifact: a.Name, Kind: a.Kind, Module: buildrun.NodeBuildModule, Language: "typescript"}) case a.Kind == catalogue.ArtifactImage: p.Tasks = append(p.Tasks, Task{Artifact: a.Name, Kind: a.Kind, Module: ImageBuildModule}) + case a.Kind == catalogue.ArtifactUpstream && !strings.Contains(a.From, "@sha256:"): + return Plan{}, &NotPinned{Artifact: a.Name, From: a.From} + case a.Kind == catalogue.ArtifactUpstream: + p.Tasks = append(p.Tasks, Task{Artifact: a.Name, Kind: a.Kind, Module: ImageBuildModule, From: a.From}) case a.Kind == catalogue.ArtifactBundle: return Plan{}, &Refused{Artifact: a.Name, What: fmt.Sprintf("bundle of language %q", a.Language)} + case a.Kind == catalogue.ArtifactPackage: + return Plan{}, &Refused{Artifact: a.Name, What: fmt.Sprintf("package of language %q", a.Language)} default: return Plan{}, &Refused{Artifact: a.Name, What: a.Kind} } diff --git a/modules/mesh-delivery-builder/internal/build/plan_test.go b/modules/mesh-delivery-builder/internal/build/plan_test.go index d6b18b2..4a70662 100644 --- a/modules/mesh-delivery-builder/internal/build/plan_test.go +++ b/modules/mesh-delivery-builder/internal/build/plan_test.go @@ -2,6 +2,8 @@ package build import ( "errors" + + "git.novox.be/novox/mesh-sdk/go/delivery" "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" "strings" "testing" @@ -41,29 +43,35 @@ func TestPlanRoutesEachKindToItsBuildModule(t *testing.T) { // 1607/AC2 } } -func TestPlanRefusesUpstreamAndPackageByName(t *testing.T) { // 1607/AC2, ADR 0478 clause 2 - for _, c := range []struct { - artifact catalogue.Artifact - kind string - }{ - {catalogue.Artifact{Name: "up", Kind: "upstream"}, "upstream"}, - {catalogue.Artifact{Name: "pkg", Kind: "package", Language: "typescript"}, "package"}, - } { - m := manifestOf(catalogue.Artifact{Name: "ok", Kind: "archive"}, c.artifact) - p, err := PlanOf(m) - if err == nil { - t.Errorf("%s: plan %+v, want a refusal", c.kind, p) - continue - } - var r *Refused - if !asRefused(err, &r) || !strings.HasPrefix(err.Error(), RefusedPrefix) || !strings.Contains(err.Error(), c.kind) || !strings.Contains(err.Error(), "build-agent") { - t.Errorf("%s: error = %v, want it to open %q and name the kind and build-agent", c.kind, err, RefusedPrefix) - } +const pinned = "nats:2.11.17-alpine@sha256:3333333333333333333333333333333333333333333333333333333333333333" + +func TestPlanSendsAPinnedUpstreamToTheImageBuildModulesCopyAndRefusesAnUnpinnedOne(t *testing.T) { // B3, ADR 0498 clause 4 + p, err := PlanOf(manifestOf(catalogue.Artifact{Name: "ok", Kind: "archive"}, catalogue.Artifact{Name: "up", Kind: "upstream", From: pinned})) + if err != nil || len(p.Tasks) != 2 { + t.Fatalf("plan %+v, error %v", p, err) + } + if up := p.Tasks[1]; up.Artifact != "up" || up.Kind != "upstream" || up.Module != ImageBuildModule || up.From != pinned || up.Language != "" { + t.Errorf("task = %+v, want the upstream sent to %s with its pinned reference and no toolchain", up, ImageBuildModule) + } + if p.NeedsToolchain(ImageBuildModule) { + t.Error("a copy needs no toolchain") + } + _, err = PlanOf(manifestOf(catalogue.Artifact{Name: "up", Kind: "upstream", From: "nats:2.11.17-alpine"})) + if err == nil || !strings.HasPrefix(err.Error(), delivery.NotPinned+"up") || !delivery.FailedUntilChanged(err.Error()) { + t.Errorf("an upstream with no digest: error = %v, want it to open %q naming the artifact", err, delivery.NotPinned) + } +} + +func TestPlanRefusesAPackageOfAnotherLanguageByName(t *testing.T) { // ADR 0498 clause 5 + _, err := PlanOf(manifestOf(catalogue.Artifact{Name: "ok", Kind: "archive"}, catalogue.Artifact{Name: "pkg", Kind: "package", Language: "python"})) + var r *Refused + if err == nil || !asRefused(err, &r) || !strings.HasPrefix(err.Error(), delivery.RefusedKind) || !strings.Contains(err.Error(), "python") || !strings.Contains(err.Error(), "pkg") { + t.Errorf("error = %v, want it to open %q naming the artifact and its language", err, delivery.RefusedKind) } } func TestPlanRefusesABundleOfAnotherLanguageAndAModuleWithNoArtifact(t *testing.T) { - if _, err := PlanOf(manifestOf(catalogue.Artifact{Name: "x", Kind: "bundle", Language: "python"})); err == nil || !strings.HasPrefix(err.Error(), RefusedPrefix) { + if _, err := PlanOf(manifestOf(catalogue.Artifact{Name: "x", Kind: "bundle", Language: "python"})); err == nil || !strings.HasPrefix(err.Error(), delivery.RefusedKind) { t.Errorf("a python bundle: error = %v, want a refusal", err) } p, err := PlanOf(manifestOf()) diff --git a/modules/mesh-delivery-builder/internal/build/run_test.go b/modules/mesh-delivery-builder/internal/build/run_test.go index 940e4fe..5a4395b 100644 --- a/modules/mesh-delivery-builder/internal/build/run_test.go +++ b/modules/mesh-delivery-builder/internal/build/run_test.go @@ -362,19 +362,19 @@ func TestATrunkImageCallCarriesPublish(t *testing.T) { // 1705b } } -func TestAnUpstreamArtifactIsStillRefusedKeyNeverBuiltAgain(t *testing.T) { // 1607/AC2, ADR 0478 clause 2 - r := newRig(t, archiveA, catalogue.Artifact{Name: "up", Kind: "upstream"}) +func TestAnUnpinnedUpstreamFailsNotPinnedAndIsNeverBuiltAgain(t *testing.T) { // B3, ADR 0498 clause 4 + r := newRig(t, archiveA, catalogue.Artifact{Name: "up", Kind: "upstream", From: "nats:2.11.17-alpine"}) v := r.consider(t, "modules/m") - if v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, RefusedPrefix) || !strings.Contains(v.Error, "upstream") || !strings.Contains(v.Error, "build-agent") { - t.Fatalf("build = %+v, want failed 'refused kind: upstream' naming build-agent", v) + if v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, delivery.NotPinned) || !strings.Contains(v.Error, "up") { + t.Fatalf("build = %+v, want failed 'not pinned:' naming the artifact", v) } if len(r.bm.calls) != 0 { - t.Errorf("%d calls, want none for a refused kind", len(r.bm.calls)) + t.Errorf("%d calls, want none for an unpinned upstream", len(r.bm.calls)) } writes := r.st.writes r.consider(t, "modules/m") if r.st.writes != writes { - t.Errorf("a refused key was written again (%d writes, was %d)", r.st.writes, writes) + t.Errorf("an unpinned upstream's key was written again (%d writes, was %d)", r.st.writes, writes) } }