diff --git a/cmd/registry-replacer/README.md b/cmd/registry-replacer/README.md index 59b764701fc..0dc2d3a45d9 100644 --- a/cmd/registry-replacer/README.md +++ b/cmd/registry-replacer/README.md @@ -5,3 +5,6 @@ A small utility used to make sure that all builds use a cluster-local registry. * Finds all ci-operator configs with at least one images directive * Downloads the corresponding Dockerfile * If it has a reference to the api.ci registry, updates the ci-operator config to replace that with a `base_image` +* If it has replacements, checks if those apply and if not, removes them +* Removes all replacements for `ocp/builder` images +* Updates the `Dockerfile` in the images config to match whats defined in the ocp-build-data repository diff --git a/cmd/registry-replacer/main.go b/cmd/registry-replacer/main.go index 3550ca75c8c..1b9e6ba5d87 100644 --- a/cmd/registry-replacer/main.go +++ b/cmd/registry-replacer/main.go @@ -26,27 +26,37 @@ import ( "sigs.k8s.io/yaml" "github.com/openshift/ci-tools/pkg/api" + "github.com/openshift/ci-tools/pkg/api/ocpbuilddata" "github.com/openshift/ci-tools/pkg/config" "github.com/openshift/ci-tools/pkg/github" + "github.com/openshift/ci-tools/pkg/steps/release" ) type options struct { - configDir string - createPR bool - githubUserName string - selfApprove bool - pruneUnusedReplacements bool - pruneOCPBuilderReplacements bool + configDir string + createPR bool + githubUserName string + selfApprove bool + ensureCorrectPromotionDockerfile bool + ocpBuildDataRepoDir string + currentRelease ocpbuilddata.MajorMinor + pruneUnusedReplacements bool + pruneOCPBuilderReplacements bool + ensureCorrectPromotionDockerfileIngoredRepos *flagutil.Strings flagutil.GitHubOptions } func gatherOptions() (*options, error) { - o := &options{} + o := &options{ensureCorrectPromotionDockerfileIngoredRepos: &flagutil.Strings{}} o.AddFlags(flag.CommandLine) flag.StringVar(&o.configDir, "config-dir", "", "The directory with the ci-operator configs") flag.BoolVar(&o.createPR, "create-pr", false, "If the tool should automatically create a PR. Requires --token-file") flag.StringVar(&o.githubUserName, "github-user-name", "openshift-bot", "Name of the github user. Required when --create-pr is set. Does nothing otherwise") flag.BoolVar(&o.selfApprove, "self-approve", false, "If the bot should self-approve its PR.") + flag.BoolVar(&o.ensureCorrectPromotionDockerfile, "ensure-correct-promotion-dockerfile", false, "If Dockerfiles used for promotion should get updated to match whats in the ocp-build-data repo") + flag.Var(o.ensureCorrectPromotionDockerfileIngoredRepos, "ensure-correct-promotion-dockerfile-ignored-repos", "Repos that are being ignored when ensuring the correct promotion dockerfile in org/repo notation. Can be passed multiple times.") + flag.StringVar(&o.ocpBuildDataRepoDir, "ocp-build-data-repo-dir", "../ocp-build-data", "The directory in which the ocp-build-data reposity is") + flag.StringVar(&o.currentRelease.Minor, "current-release-minor", "6", "The minor version of the current release that is getting forwarded to from the master branch") flag.BoolVar(&o.pruneUnusedReplacements, "prune-unused-replacements", false, "If replacements that match nothing should get pruned from the config") flag.BoolVar(&o.pruneOCPBuilderReplacements, "prune-ocp-builder-replacements", false, "If all replacements that target the ocp/builder imagestream should be removed") flag.Parse() @@ -63,6 +73,16 @@ func gatherOptions() (*options, error) { errs = append(errs, o.GitHubOptions.Validate(false)) } + if o.ensureCorrectPromotionDockerfile { + if o.ocpBuildDataRepoDir == "" { + errs = append(errs, errors.New("--ocp-build-data-repo-dir must be set when --ensure-correct-promotion-dockerfile is set")) + } + if o.currentRelease.Minor == "" { + errs = append(errs, errors.New("--current-release must be set when --ensure-correct-promotion-dockerfile is set")) + } + o.currentRelease.Major = "4" + } + return o, utilerrors.NewAggregate(errs) } @@ -86,6 +106,15 @@ func main() { } } + var promotionTargetToDockerfileMapping map[string]dockerfileLocation + if opts.ensureCorrectPromotionDockerfile { + var err error + promotionTargetToDockerfileMapping, err = getPromotionTargetToDockerfileMapping(opts.ocpBuildDataRepoDir, opts.currentRelease) + if err != nil { + logrus.WithError(err).Fatal("Failed to construct promotion target to dockerfile mapping") + } + } + var errs []error errLock := &sync.Mutex{} wg := sync.WaitGroup{} @@ -102,6 +131,10 @@ func main() { }, opts.pruneUnusedReplacements, opts.pruneOCPBuilderReplacements, + opts.ensureCorrectPromotionDockerfile, + sets.NewString(opts.ensureCorrectPromotionDockerfileIngoredRepos.Strings()...), + promotionTargetToDockerfileMapping, + opts.currentRelease, )(config, info); err != nil { errLock.Lock() errs = append(errs, err) @@ -135,6 +168,10 @@ func replacer( writer func([]byte) error, pruneUnusedReplacementsEnabled bool, pruneOCPBuilderReplacementsEnabled bool, + ensureCorrectPromotionDockerfile bool, + ensureCorrectPromotionDockerfileIgnoredrepos sets.String, + promotionTargetToDockerfileMapping map[string]dockerfileLocation, + majorMinor ocpbuilddata.MajorMinor, ) func(*api.ReleaseBuildConfiguration, *config.Info) error { return func(config *api.ReleaseBuildConfiguration, info *config.Info) error { if len(config.Images) == 0 { @@ -146,6 +183,12 @@ func replacer( return fmt.Errorf("failed to marshal config for comparison: %w", err) } + // We have to do this first because the result of the following operations might + // change based on what we do here. + if ensureCorrectPromotionDockerfile { + updateDockerfilesToMatchOCPBuildData(config, promotionTargetToDockerfileMapping, majorMinor.String(), ensureCorrectPromotionDockerfileIgnoredrepos) + } + getter := githubFileGetterFactory(info.Org, info.Repo, info.Branch) allReplacementCandidates := sets.String{} @@ -509,3 +552,67 @@ func pruneReplacements(config *api.ReleaseBuildConfiguration, filter asDirective return utilerrors.NewAggregate(errs) } + +type dockerfileLocation struct { + contextDir string + dockerfile string +} + +func getPromotionTargetToDockerfileMapping(ocpBuildDataDir string, majorMinor ocpbuilddata.MajorMinor) (map[string]dockerfileLocation, error) { + configs, err := ocpbuilddata.LoadImageConfigs(ocpBuildDataDir, majorMinor) + if err != nil { + return nil, fmt.Errorf("failed to read image configs from ocp-build-data: %w", err) + } + result := map[string]dockerfileLocation{} + for _, config := range configs { + result[config.PromotesTo()] = dockerfileLocation{contextDir: config.Content.Source.Path, dockerfile: config.Content.Source.Dockerfile} + } + return result, nil +} + +func updateDockerfilesToMatchOCPBuildData( + config *api.ReleaseBuildConfiguration, + promotionTargetToDockerfileMapping map[string]dockerfileLocation, + majorMinorVersion string, + ignoredRepos sets.String, +) { + + // The tool only works for the current release + if config.Metadata.Branch != "master" { + return + } + if ignoredRepos.Has(config.Metadata.Org + "/" + config.Metadata.Repo) { + return + } + + // Configs indexed by tag + promotedTags := map[string]api.ImageStreamTagReference{} + for _, promotedTag := range release.PromotedTags(config) { + if promotedTag.Namespace != "ocp" || promotedTag.Name != majorMinorVersion { + continue + } + promotedTags[promotedTag.Tag] = promotedTag + } + if len(promotedTags) == 0 { + return + } + + for idx, image := range config.Images { + promotionTarget, ok := promotedTags[string(image.To)] + if !ok { + continue + } + stringifiedPromotionTarget := fmt.Sprintf("registry.svc.ci.openshift.org/%s/%s:%s", promotionTarget.Namespace, promotionTarget.Name, image.To) + dockerfilePath, ok := promotionTargetToDockerfileMapping[stringifiedPromotionTarget] + if !ok { + logrus.WithField("promotiontarget", stringifiedPromotionTarget).Info("Ignoring promotion target for which we have no ocp-build-data config") + continue + } + if image.ContextDir != dockerfilePath.contextDir { + config.Images[idx].ContextDir = dockerfilePath.contextDir + } + if image.DockerfilePath != dockerfilePath.dockerfile { + config.Images[idx].DockerfilePath = dockerfilePath.dockerfile + } + } +} diff --git a/cmd/registry-replacer/main_test.go b/cmd/registry-replacer/main_test.go index b441ff4bc6e..1a8494dfc7a 100644 --- a/cmd/registry-replacer/main_test.go +++ b/cmd/registry-replacer/main_test.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -8,19 +9,24 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "github.com/openshift/ci-tools/pkg/api" + "github.com/openshift/ci-tools/pkg/api/ocpbuilddata" "github.com/openshift/ci-tools/pkg/config" "github.com/openshift/ci-tools/pkg/github" "github.com/openshift/ci-tools/pkg/testhelper" ) func TestReplacer(t *testing.T) { + majorMinor := ocpbuilddata.MajorMinor{Major: "4", Minor: "6"} testCases := []struct { - name string - config *api.ReleaseBuildConfiguration - pruneUnusedReplacementsEnabled bool - pruneOCPBuilderReplacementsEnabled bool - files map[string][]byte - expectWrite bool + name string + config *api.ReleaseBuildConfiguration + pruneUnusedReplacementsEnabled bool + pruneOCPBuilderReplacementsEnabled bool + ensureCorrectPromotionDockerfile bool + ensureCorrectPromotionDockerfileIngoredRepos sets.String + promotionTargetToDockerfileMapping map[string]dockerfileLocation + files map[string][]byte + expectWrite bool }{ { name: "No dockerfile, does nothing", @@ -143,6 +149,117 @@ func TestReplacer(t *testing.T) { pruneOCPBuilderReplacementsEnabled: true, expectWrite: true, }, + { + name: "Dockerfile gets fixed up", + config: &api.ReleaseBuildConfiguration{ + Images: []api.ProjectDirectoryImageBuildStepConfiguration{{ + ProjectDirectoryImageBuildInputs: api.ProjectDirectoryImageBuildInputs{ + Inputs: map[string]api.ImageBuildInputs{ + "root": {As: []string{"ocp/builder:something"}}, + }, + }, + To: "promotionTarget", + }}, + PromotionConfiguration: &api.PromotionConfiguration{Namespace: "ocp", Name: majorMinor.String()}, + Metadata: api.Metadata{Branch: "master"}, + }, + ensureCorrectPromotionDockerfile: true, + promotionTargetToDockerfileMapping: map[string]dockerfileLocation{fmt.Sprintf("registry.svc.ci.openshift.org/ocp/%s:promotionTarget", majorMinor.String()): {dockerfile: "Dockerfile.rhel"}}, + expectWrite: true, + }, + { + name: "Config for non-master branch is ignored", + config: &api.ReleaseBuildConfiguration{ + Images: []api.ProjectDirectoryImageBuildStepConfiguration{{ + ProjectDirectoryImageBuildInputs: api.ProjectDirectoryImageBuildInputs{ + Inputs: map[string]api.ImageBuildInputs{ + "root": {As: []string{"ocp/builder:something"}}, + }, + }, + To: "promotionTarget", + }}, + PromotionConfiguration: &api.PromotionConfiguration{Namespace: "ocp", Name: majorMinor.String()}, + }, + ensureCorrectPromotionDockerfile: true, + promotionTargetToDockerfileMapping: map[string]dockerfileLocation{fmt.Sprintf("registry.svc.ci.openshift.org/ocp/%s:promotionTarget", majorMinor.String()): {dockerfile: "Dockerfile.rhel"}}, + }, + { + name: "Dockerfile is correct, nothing to do", + config: &api.ReleaseBuildConfiguration{ + Images: []api.ProjectDirectoryImageBuildStepConfiguration{{ + ProjectDirectoryImageBuildInputs: api.ProjectDirectoryImageBuildInputs{ + DockerfilePath: "Dockerfile.rhel", + Inputs: map[string]api.ImageBuildInputs{ + "root": {As: []string{"ocp/builder:something"}}, + }, + }, + To: "promotionTarget", + }}, + PromotionConfiguration: &api.PromotionConfiguration{Namespace: "ocp", Name: majorMinor.String()}, + Metadata: api.Metadata{Branch: "master"}, + }, + ensureCorrectPromotionDockerfile: true, + promotionTargetToDockerfileMapping: map[string]dockerfileLocation{fmt.Sprintf("registry.svc.ci.openshift.org/ocp/%s:promotionTarget", majorMinor.String()): {dockerfile: "Dockerfile.rhel"}}, + }, + { + name: "Context dir gets fixed up", + config: &api.ReleaseBuildConfiguration{ + Images: []api.ProjectDirectoryImageBuildStepConfiguration{{ + ProjectDirectoryImageBuildInputs: api.ProjectDirectoryImageBuildInputs{ + ContextDir: "some-dir", + DockerfilePath: "Dockerfile.rhel", + Inputs: map[string]api.ImageBuildInputs{ + "root": {As: []string{"ocp/builder:something"}}, + }, + }, + To: "promotionTarget", + }}, + PromotionConfiguration: &api.PromotionConfiguration{Namespace: "ocp", Name: majorMinor.String()}, + Metadata: api.Metadata{Branch: "master"}, + }, + ensureCorrectPromotionDockerfile: true, + promotionTargetToDockerfileMapping: map[string]dockerfileLocation{fmt.Sprintf("registry.svc.ci.openshift.org/ocp/%s:promotionTarget", majorMinor.String()): {contextDir: "other_dir", dockerfile: "Dockerfile.rhel"}}, + expectWrite: true, + }, + { + name: "Context dir is ncorrect, but ignored", + config: &api.ReleaseBuildConfiguration{ + Images: []api.ProjectDirectoryImageBuildStepConfiguration{{ + ProjectDirectoryImageBuildInputs: api.ProjectDirectoryImageBuildInputs{ + ContextDir: "some-dir", + DockerfilePath: "Dockerfile.rhel", + Inputs: map[string]api.ImageBuildInputs{ + "root": {As: []string{"ocp/builder:something"}}, + }, + }, + To: "promotionTarget", + }}, + PromotionConfiguration: &api.PromotionConfiguration{Namespace: "ocp", Name: majorMinor.String()}, + Metadata: api.Metadata{Branch: "master", Org: "org", Repo: "repo"}, + }, + ensureCorrectPromotionDockerfile: true, + ensureCorrectPromotionDockerfileIngoredRepos: sets.NewString("org/repo"), + promotionTargetToDockerfileMapping: map[string]dockerfileLocation{fmt.Sprintf("registry.svc.ci.openshift.org/ocp/%s:promotionTarget", majorMinor.String()): {contextDir: "other_dir", dockerfile: "Dockerfile.rhel"}}, + }, + { + name: "Dockerfile+Context dir is correct, nothing to do", + config: &api.ReleaseBuildConfiguration{ + Images: []api.ProjectDirectoryImageBuildStepConfiguration{{ + ProjectDirectoryImageBuildInputs: api.ProjectDirectoryImageBuildInputs{ + ContextDir: "some_dir", + DockerfilePath: "Dockerfile.rhel", + Inputs: map[string]api.ImageBuildInputs{ + "root": {As: []string{"ocp/builder:something"}}, + }, + }, + To: "promotionTarget", + }}, + PromotionConfiguration: &api.PromotionConfiguration{Namespace: "ocp", Name: majorMinor.String()}, + Metadata: api.Metadata{Branch: "master"}, + }, + ensureCorrectPromotionDockerfile: true, + promotionTargetToDockerfileMapping: map[string]dockerfileLocation{fmt.Sprintf("registry.svc.ci.openshift.org/ocp/%s:promotionTarget", majorMinor.String()): {contextDir: "some_dir", dockerfile: "Dockerfile.rhel"}}, + }, } for _, tc := range testCases { @@ -151,7 +268,16 @@ func TestReplacer(t *testing.T) { t.Parallel() fakeWriter := &fakeWriter{} - if err := replacer(fakeGithubFileGetterFactory(tc.files), fakeWriter.Write, tc.pruneUnusedReplacementsEnabled, tc.pruneOCPBuilderReplacementsEnabled)(tc.config, &config.Info{}); err != nil { + if err := replacer( + fakeGithubFileGetterFactory(tc.files), + fakeWriter.Write, + tc.pruneUnusedReplacementsEnabled, + tc.pruneOCPBuilderReplacementsEnabled, + tc.ensureCorrectPromotionDockerfile, + tc.ensureCorrectPromotionDockerfileIngoredRepos, + tc.promotionTargetToDockerfileMapping, + majorMinor, + )(tc.config, &config.Info{}); err != nil { t.Errorf("replacer failed: %v", err) } if (fakeWriter.data != nil) != tc.expectWrite { diff --git a/cmd/registry-replacer/testdata/zz_fixture_TestReplacer_Context_dir_gets_fixed_up.yaml b/cmd/registry-replacer/testdata/zz_fixture_TestReplacer_Context_dir_gets_fixed_up.yaml new file mode 100644 index 00000000000..5af036220e3 --- /dev/null +++ b/cmd/registry-replacer/testdata/zz_fixture_TestReplacer_Context_dir_gets_fixed_up.yaml @@ -0,0 +1,15 @@ +images: +- context_dir: other_dir + dockerfile_path: Dockerfile.rhel + inputs: + root: + as: + - ocp/builder:something + to: promotionTarget +promotion: + name: "4.6" + namespace: ocp +zz_generated_metadata: + branch: master + org: "" + repo: "" diff --git a/cmd/registry-replacer/testdata/zz_fixture_TestReplacer_Dockerfile_gets_fixed_up.yaml b/cmd/registry-replacer/testdata/zz_fixture_TestReplacer_Dockerfile_gets_fixed_up.yaml new file mode 100644 index 00000000000..c0a7c4abcd7 --- /dev/null +++ b/cmd/registry-replacer/testdata/zz_fixture_TestReplacer_Dockerfile_gets_fixed_up.yaml @@ -0,0 +1,14 @@ +images: +- dockerfile_path: Dockerfile.rhel + inputs: + root: + as: + - ocp/builder:something + to: promotionTarget +promotion: + name: "4.6" + namespace: ocp +zz_generated_metadata: + branch: master + org: "" + repo: "" diff --git a/pkg/api/ocpbuilddata/types.go b/pkg/api/ocpbuilddata/types.go index f63f06f3f7d..7816af1674d 100644 --- a/pkg/api/ocpbuilddata/types.go +++ b/pkg/api/ocpbuilddata/types.go @@ -312,6 +312,10 @@ type MajorMinor struct { Minor string } +func (mm MajorMinor) String() string { + return mm.Major + "." + mm.Minor +} + func gatherAllOCPImageConfigs(ocpBuildDataDir string, majorMinor MajorMinor) (map[string]OCPImageConfig, error) { result := map[string]OCPImageConfig{} resultLock := &sync.Mutex{}