From 50b32dd801eb675f403832365dd0725ba1a0140a Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Sun, 3 Mar 2019 16:22:37 -0500 Subject: [PATCH] Remove support for release configurations not in a single stream We don't use this code and are unlikely to at any point in the future. Clean up these code paths prior to supporting upgrade logic. --- CONFIGURATION.md | 22 +-- pkg/api/config.go | 14 +- pkg/api/types.go | 26 +--- pkg/defaults/defaults.go | 27 ++-- pkg/steps/release/promote.go | 129 +++++------------ pkg/steps/release/release_images.go | 205 ++++++++-------------------- 6 files changed, 116 insertions(+), 307 deletions(-) diff --git a/CONFIGURATION.md b/CONFIGURATION.md index 009f86d2..8c0c54fc 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -97,22 +97,12 @@ tag_specification: cluster: https://api.ci.openshift.org name: origin-v3.11 namespace: openshift - tag: '' tag_overrides: {} ``` -There are two primary modes for assembling a release: - - single `ImageStream`, multiple tags (`openshift/origin-v3.9:control-plane`) - - multiple `ImageStream`s, one tag (`openshift/origin-control-plane:v3.9`) - -The former works well for central control, the latter for distributed control: -when many disparate processes are publishing images at their own cadences, each -process can own its own `ImageStream` and coordination between processes can be -through coordination in `ImageStreamTag`s. When one process marshalls a release, -the process can push to one `ImageStream` under multiple `ImageStreamTags`. In -practice, the OpenShift releases are assembled using the former approach, while -non-release images like infrastructure tooling, _etc_, are assembled using the -latter. +The release tag specification points to an image stream containing multiple tags, +each of which references a single component by a well known name, e.g. +`openshift/origin-v3.9:control-plane`. ## `tag_specification.cluster` `cluster` is an optional cluster string (`host`, `host:port`, or `scheme://host:port`) @@ -120,16 +110,12 @@ to connect to for the `ImageStream`. The referenced OpenShift cluster must suppo anonymous access to retrieve `ImageStream`s, `ImageStreamTag`s, and `ImageStreamImage`s in the provided namespace. -## `tag_specification.tag` -`tag` is used to specify the single tag when multiple `ImageStreams` but one tag -are used to assemble a release. - ## `tag_specification.namespace` `namespace` determines the `Namespace` on the target cluster where release `ImageStreams` are located. ## `tag_specification.name` -`name` is the `ImageStream` name when a single `ImageStream` but multiple +`name` is the `ImageStream` name where a single `ImageStream` and multiple tags are used to assemble a release. ## `tag_specification.tag_overrides` diff --git a/pkg/api/config.go b/pkg/api/config.go index 8e14301b..f76464ac 100644 --- a/pkg/api/config.go +++ b/pkg/api/config.go @@ -62,11 +62,11 @@ func validatePromotionWithTagSpec(promotion *PromotionConfiguration, tagSpec *Re if len(promotion.Namespace) == 0 && len(tagSpec.Namespace) == 0 { validationErrors = append(validationErrors, fmt.Errorf("promotion: no namespace defined")) } - if len(promotion.Name) == 0 && len(promotion.Tag) == 0 { - if len(tagSpec.Name) != 0 || len(tagSpec.Tag) != 0 { + if len(promotion.Name) == 0 { + if len(tagSpec.Name) != 0 { // will get defaulted, is ok } else { - validationErrors = append(validationErrors, errors.New("promotion: no name or tag provided and could not derive defaults from tag_specification")) + validationErrors = append(validationErrors, errors.New("promotion: no name provided and could not derive defaults from tag_specification")) } } @@ -156,8 +156,8 @@ func validatePromotionConfiguration(fieldRoot string, input PromotionConfigurati validationErrors = append(validationErrors, fmt.Errorf("%s: no namespace defined", fieldRoot)) } - if len(input.Name) == 0 && len(input.Tag) == 0 { - validationErrors = append(validationErrors, fmt.Errorf("%s: no name or tag defined", fieldRoot)) + if len(input.Name) == 0 { + validationErrors = append(validationErrors, fmt.Errorf("%s: no name defined", fieldRoot)) } return validationErrors } @@ -169,8 +169,8 @@ func validateReleaseTagConfiguration(fieldRoot string, input ReleaseTagConfigura validationErrors = append(validationErrors, fmt.Errorf("%s: no namespace defined", fieldRoot)) } - if len(input.Name) == 0 && len(input.Tag) == 0 { - validationErrors = append(validationErrors, fmt.Errorf("%s: no name or tag defined", fieldRoot)) + if len(input.Name) == 0 { + validationErrors = append(validationErrors, fmt.Errorf("%s: no name defined", fieldRoot)) } return validationErrors } diff --git a/pkg/api/types.go b/pkg/api/types.go index 134d974e..039ef0ff 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -152,11 +152,9 @@ type ImageStreamTagReference struct { } // ReleaseTagConfiguration describes how a release is -// assembled from release artifacts. There are two primary modes, -// single stream, multiple tags (openshift/origin-v3.9:control-plane) -// on one stream, or multiple streams with one tag -// (openshift/origin-control-plane:v3.9). The former works well for -// central control, the latter for distributed control. +// assembled from release artifacts. A release image stream is a +// single stream with multiple tags (openshift/origin-v3.9:control-plane), +// each tag being a unique and well defined name for a component. type ReleaseTagConfiguration struct { // Cluster is an optional cluster string (host, host:port, or // scheme://host:port) to connect to for this image stream. The @@ -170,15 +168,10 @@ type ReleaseTagConfiguration struct { // job are tagged from. Namespace string `json:"namespace"` - // Name is an optional image stream name to use that - // contains all component tags. If specified, tag is - // ignored. + // Name is the image stream name to use that contains all + // component tags. Name string `json:"name"` - // Tag is the ImageStreamTag tagged in for each - // ImageStream in the above Namespace. - Tag string `json:"tag,omitempty"` - // NamePrefix is prepended to the final output image name // if specified. NamePrefix string `json:"name_prefix,omitempty"` @@ -198,15 +191,10 @@ type PromotionConfiguration struct { // artifacts will be published to. Namespace string `json:"namespace"` - // Name is an optional image stream name to use that - // contains all component tags. If specified, tag is - // ignored. + // Name is the image stream name to use that + // contains all component tags. Name string `json:"name"` - // Tag is the ImageStreamTag tagged in for each - // build image's ImageStream. - Tag string `json:"tag,omitempty"` - // NamePrefix is prepended to the final output image name // if specified. NamePrefix string `json:"name_prefix,omitempty"` diff --git a/pkg/defaults/defaults.go b/pkg/defaults/defaults.go index d00ab205..bd13c1e7 100644 --- a/pkg/defaults/defaults.go +++ b/pkg/defaults/defaults.go @@ -361,25 +361,14 @@ func stepConfigsForBuild(config *api.ReleaseBuildConfiguration, jobSpec *api.Job image := &config.Images[i] buildSteps = append(buildSteps, api.StepConfiguration{ProjectDirectoryImageBuildStepConfiguration: image}) if config.ReleaseTagConfiguration != nil { - if len(config.ReleaseTagConfiguration.Name) > 0 { - buildSteps = append(buildSteps, api.StepConfiguration{OutputImageTagStepConfiguration: &api.OutputImageTagStepConfiguration{ - From: image.To, - To: api.ImageStreamTagReference{ - Name: fmt.Sprintf("%s%s", config.ReleaseTagConfiguration.NamePrefix, api.StableImageStream), - Tag: string(image.To), - }, - Optional: image.Optional, - }}) - } else { - buildSteps = append(buildSteps, api.StepConfiguration{OutputImageTagStepConfiguration: &api.OutputImageTagStepConfiguration{ - From: image.To, - To: api.ImageStreamTagReference{ - Name: string(image.To), - Tag: "ci", - }, - Optional: image.Optional, - }}) - } + buildSteps = append(buildSteps, api.StepConfiguration{OutputImageTagStepConfiguration: &api.OutputImageTagStepConfiguration{ + From: image.To, + To: api.ImageStreamTagReference{ + Name: fmt.Sprintf("%s%s", config.ReleaseTagConfiguration.NamePrefix, api.StableImageStream), + Tag: string(image.To), + }, + Optional: image.Optional, + }}) } else { buildSteps = append(buildSteps, api.StepConfiguration{OutputImageTagStepConfiguration: &api.OutputImageTagStepConfiguration{ From: image.To, diff --git a/pkg/steps/release/promote.go b/pkg/steps/release/promote.go index 59610994..dd9520fe 100644 --- a/pkg/steps/release/promote.go +++ b/pkg/steps/release/promote.go @@ -31,10 +31,7 @@ type promotionStep struct { } func targetName(config api.PromotionConfiguration) string { - if len(config.Name) > 0 { - return fmt.Sprintf("%s/%s:${component}", config.Namespace, config.Name) - } - return fmt.Sprintf("%s/${component}:%s", config.Namespace, config.Tag) + return fmt.Sprintf("%s/%s:%s", config.Namespace, config.Name, api.ComponentFormatReplacement) } func (s *promotionStep) Inputs(ctx context.Context, dry bool) (api.InputDefinition, error) { @@ -71,107 +68,49 @@ func (s *promotionStep) Run(ctx context.Context, dry bool) error { return fmt.Errorf("could not resolve pipeline imagestream: %v", err) } - if len(s.config.Name) > 0 { - return retry.RetryOnConflict(promotionRetry, func() error { - is, err := s.dstClient.ImageStreams(s.config.Namespace).Get(s.config.Name, meta.GetOptions{}) - if errors.IsNotFound(err) { - is, err = s.dstClient.ImageStreams(s.config.Namespace).Create(&imageapi.ImageStream{ - ObjectMeta: meta.ObjectMeta{ - Name: s.config.Name, - Namespace: s.config.Namespace, - }, - }) - } - if err != nil { - return fmt.Errorf("could not retrieve target imagestream: %v", err) - } - - for dst, src := range tags { - if valid, _ := findStatusTag(pipeline, src); valid != nil { - is.Spec.Tags = append(is.Spec.Tags, imageapi.TagReference{ - Name: dst, - From: valid, - }) - } - } - - if dry { - istJSON, err := json.MarshalIndent(is, "", " ") - if err != nil { - return fmt.Errorf("failed to marshal image stream: %v", err) - } - fmt.Printf("%s\n", istJSON) - return nil - } - if _, err := s.dstClient.ImageStreams(s.config.Namespace).Update(is); err != nil { - if errors.IsConflict(err) { - return err - } - return fmt.Errorf("could not promote image streams: %v", err) - } - return nil - }) + if len(s.config.Name) == 0 { + return fmt.Errorf("name is a required field for release tagging") } - client := s.dstClient.ImageStreamTags(s.config.Namespace) - for dst, src := range tags { - valid, _ := findStatusTag(pipeline, src) - if valid == nil { - continue - } - - name := fmt.Sprintf("%s%s", s.config.NamePrefix, dst) - - err := retry.RetryOnConflict(promotionRetry, func() error { - _, err := s.dstClient.ImageStreams(s.config.Namespace).Get(name, meta.GetOptions{}) - if errors.IsNotFound(err) { - _, err = s.dstClient.ImageStreams(s.config.Namespace).Create(&imageapi.ImageStream{ - ObjectMeta: meta.ObjectMeta{ - Name: name, - Namespace: s.config.Namespace, - }, - Spec: imageapi.ImageStreamSpec{ - LookupPolicy: imageapi.ImageLookupPolicy{ - Local: true, - }, - }, - }) - } - if err != nil { - return fmt.Errorf("could not ensure target imagestream: %v", err) - } - - ist := &imageapi.ImageStreamTag{ + return retry.RetryOnConflict(promotionRetry, func() error { + is, err := s.dstClient.ImageStreams(s.config.Namespace).Get(s.config.Name, meta.GetOptions{}) + if errors.IsNotFound(err) { + is, err = s.dstClient.ImageStreams(s.config.Namespace).Create(&imageapi.ImageStream{ ObjectMeta: meta.ObjectMeta{ - Name: fmt.Sprintf("%s:%s", name, s.config.Tag), + Name: s.config.Name, Namespace: s.config.Namespace, }, - Tag: &imageapi.TagReference{ - Name: s.config.Tag, + }) + } + if err != nil { + return fmt.Errorf("could not retrieve target imagestream: %v", err) + } + + for dst, src := range tags { + if valid, _ := findStatusTag(pipeline, src); valid != nil { + is.Spec.Tags = append(is.Spec.Tags, imageapi.TagReference{ + Name: dst, From: valid, - }, - } - if dry { - istJSON, err := json.MarshalIndent(ist, "", " ") - if err != nil { - return fmt.Errorf("failed to marshal imagestreamtag: %v", err) - } - fmt.Printf("%s\n", istJSON) - return nil + }) } - if _, err := client.Update(ist); err != nil { - if errors.IsConflict(err) { - return err - } - return fmt.Errorf("could not promote imagestreamtag %s: %v", dst, err) + } + + if dry { + istJSON, err := json.MarshalIndent(is, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal image stream: %v", err) } + fmt.Printf("%s\n", istJSON) return nil - }) - if err != nil { - return err } - } - return nil + if _, err := s.dstClient.ImageStreams(s.config.Namespace).Update(is); err != nil { + if errors.IsConflict(err) { + return err + } + return fmt.Errorf("could not promote image streams: %v", err) + } + return nil + }) } func (s *promotionStep) Done() (bool, error) { diff --git a/pkg/steps/release/release_images.go b/pkg/steps/release/release_images.go index 62429578..705671d9 100644 --- a/pkg/steps/release/release_images.go +++ b/pkg/steps/release/release_images.go @@ -35,7 +35,7 @@ func StableImagesTagStep(dstClient imageclientset.ImageV1Interface, jobSpec *api } func (s *stableImagesTagStep) Run(ctx context.Context, dry bool) error { - log.Printf("Will output images to %s:${component}", api.StableImageStream) + log.Printf("Will output images to %s:%s", api.StableImageStream, api.ComponentFormatReplacement) newIS := &imageapi.ImageStream{ ObjectMeta: meta.ObjectMeta{ @@ -122,10 +122,7 @@ func (s *releaseImagesTagStep) Inputs(ctx context.Context, dry bool) (api.InputD } func sourceName(config api.ReleaseTagConfiguration) string { - if len(config.Name) > 0 { - return fmt.Sprintf("%s/%s:${component}", config.Namespace, config.Name) - } - return fmt.Sprintf("%s/${component}:%s", config.Namespace, config.Tag) + return fmt.Sprintf("%s/%s:%s", config.Namespace, config.Name, api.ComponentFormatReplacement) } func (s *releaseImagesTagStep) Run(ctx context.Context, dry bool) error { @@ -139,160 +136,75 @@ func (s *releaseImagesTagStep) Run(ctx context.Context, dry bool) error { } } - if len(s.config.Name) > 0 { - is, err := s.srcClient.ImageStreams(s.config.Namespace).Get(s.config.Name, meta.GetOptions{}) - if err != nil { - return fmt.Errorf("could not resolve stable imagestream: %v", err) - } + is, err := s.srcClient.ImageStreams(s.config.Namespace).Get(s.config.Name, meta.GetOptions{}) + if err != nil { + return fmt.Errorf("could not resolve stable imagestream: %v", err) + } - // check to see if the src and dst are the same cluster, in which case we can use a more efficient tagging path - if len(s.config.Cluster) > 0 { - if dstIs, err := s.dstClient.ImageStreams(is.Namespace).Get(is.Name, meta.GetOptions{}); err == nil && dstIs.UID == is.UID { - s.config.Cluster = "" - } + // check to see if the src and dst are the same cluster, in which case we can use a more efficient tagging path + if len(s.config.Cluster) > 0 { + if dstIs, err := s.dstClient.ImageStreams(is.Namespace).Get(is.Name, meta.GetOptions{}); err == nil && dstIs.UID == is.UID { + s.config.Cluster = "" } + } - var repo string - if len(s.config.Cluster) > 0 { - if len(is.Status.PublicDockerImageRepository) > 0 { - repo = is.Status.PublicDockerImageRepository - } else if len(is.Status.DockerImageRepository) > 0 { - repo = is.Status.DockerImageRepository - } else { - return fmt.Errorf("remote image stream %s has no accessible image registry value", s.config.Name) - } + var repo string + if len(s.config.Cluster) > 0 { + if len(is.Status.PublicDockerImageRepository) > 0 { + repo = is.Status.PublicDockerImageRepository + } else if len(is.Status.DockerImageRepository) > 0 { + repo = is.Status.DockerImageRepository + } else { + return fmt.Errorf("remote image stream %s has no accessible image registry value", s.config.Name) } + } - is.UID = "" - newIS := &imageapi.ImageStream{ - ObjectMeta: meta.ObjectMeta{ - Name: api.StableImageStream, - }, - Spec: imageapi.ImageStreamSpec{ - LookupPolicy: imageapi.ImageLookupPolicy{ - Local: true, - }, + is.UID = "" + newIS := &imageapi.ImageStream{ + ObjectMeta: meta.ObjectMeta{ + Name: api.StableImageStream, + }, + Spec: imageapi.ImageStreamSpec{ + LookupPolicy: imageapi.ImageLookupPolicy{ + Local: true, }, - } - for _, tag := range is.Spec.Tags { - if valid, image := findStatusTag(is, tag.Name); valid != nil { - if len(s.config.Cluster) > 0 { - if len(image) > 0 { - valid = &coreapi.ObjectReference{Kind: "DockerImage", Name: fmt.Sprintf("%s@%s", repo, image)} - } else { - valid = &coreapi.ObjectReference{Kind: "DockerImage", Name: fmt.Sprintf("%s:%s", repo, tag.Name)} - } + }, + } + for _, tag := range is.Spec.Tags { + if valid, image := findStatusTag(is, tag.Name); valid != nil { + if len(s.config.Cluster) > 0 { + if len(image) > 0 { + valid = &coreapi.ObjectReference{Kind: "DockerImage", Name: fmt.Sprintf("%s@%s", repo, image)} + } else { + valid = &coreapi.ObjectReference{Kind: "DockerImage", Name: fmt.Sprintf("%s:%s", repo, tag.Name)} } - newIS.Spec.Tags = append(newIS.Spec.Tags, imageapi.TagReference{ - Name: tag.Name, - From: valid, - }) } + newIS.Spec.Tags = append(newIS.Spec.Tags, imageapi.TagReference{ + Name: tag.Name, + From: valid, + }) } + } - if dry { - istJSON, err := json.MarshalIndent(newIS, "", " ") - if err != nil { - return fmt.Errorf("failed to marshal image stream: %v", err) - } - fmt.Printf("%s\n", istJSON) - return nil - } - is, err = s.dstClient.ImageStreams(s.jobSpec.Namespace).Create(newIS) - if err != nil && !errors.IsAlreadyExists(err) { - return fmt.Errorf("could not copy stable imagestreamtag: %v", err) - } - - for _, tag := range is.Spec.Tags { - spec, ok := resolvePullSpec(is, tag.Name, false) - if !ok { - continue - } - s.params.Set(componentToParamName(tag.Name), spec) + if dry { + istJSON, err := json.MarshalIndent(newIS, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal image stream: %v", err) } - + fmt.Printf("%s\n", istJSON) return nil } - - stableImageStreams, err := s.srcClient.ImageStreams(s.config.Namespace).List(meta.ListOptions{}) - if err != nil { - return fmt.Errorf("could not resolve stable imagestreams: %v", err) + is, err = s.dstClient.ImageStreams(s.jobSpec.Namespace).Create(newIS) + if err != nil && !errors.IsAlreadyExists(err) { + return fmt.Errorf("could not copy stable imagestreamtag: %v", err) } - for i, stableImageStream := range stableImageStreams.Items { - log.Printf("Considering stable image stream %s", stableImageStream.Name) - targetTag := s.config.Tag - if override, ok := s.config.TagOverrides[stableImageStream.Name]; ok { - targetTag = override - } - - // check exactly once to see if the src and dst are the same cluster, in which case we can use a more efficient tagging path - if i == 0 && len(s.config.Cluster) > 0 { - if dstIs, err := s.dstClient.ImageStreams(stableImageStream.Namespace).Get(stableImageStream.Name, meta.GetOptions{}); err == nil && dstIs.UID == stableImageStream.UID { - s.config.Cluster = "" - } - } - - var repo string - if len(s.config.Cluster) > 0 { - if len(stableImageStream.Status.PublicDockerImageRepository) > 0 { - repo = stableImageStream.Status.PublicDockerImageRepository - } else if len(stableImageStream.Status.DockerImageRepository) > 0 { - repo = stableImageStream.Status.DockerImageRepository - } else { - return fmt.Errorf("remote image stream %s has no accessible image registry value", s.config.Name) - } - } - - for _, tag := range stableImageStream.Spec.Tags { - if tag.Name == targetTag { - log.Printf("Cross-tagging %s:%s from %s/%s:%s", stableImageStream.Name, targetTag, stableImageStream.Namespace, stableImageStream.Name, targetTag) - var id string - for _, tagStatus := range stableImageStream.Status.Tags { - if tagStatus.Tag == targetTag { - id = tagStatus.Items[0].Image - } - } - if len(id) == 0 { - return fmt.Errorf("no image found backing %s/%s:%s", stableImageStream.Namespace, stableImageStream.Name, targetTag) - } - ist := &imageapi.ImageStreamTag{ - ObjectMeta: meta.ObjectMeta{ - Namespace: s.jobSpec.Namespace, - Name: fmt.Sprintf("%s:%s", stableImageStream.Name, targetTag), - }, - Tag: &imageapi.TagReference{ - Name: targetTag, - From: &coreapi.ObjectReference{ - Kind: "ImageStreamImage", - Name: fmt.Sprintf("%s@%s", stableImageStream.Name, id), - Namespace: s.config.Namespace, - }, - }, - } - - if len(s.config.Cluster) > 0 { - ist.Tag.From = &coreapi.ObjectReference{Kind: "DockerImage", Name: fmt.Sprintf("%s@%s", repo, id)} - } - - if dry { - istJSON, err := json.MarshalIndent(ist, "", " ") - if err != nil { - return fmt.Errorf("failed to marshal imagestreamtag: %v", err) - } - fmt.Printf("%s\n", istJSON) - continue - } - ist, err := s.dstClient.ImageStreamTags(s.jobSpec.Namespace).Create(ist) - if err != nil && !errors.IsAlreadyExists(err) { - return fmt.Errorf("could not copy stable imagestreamtag: %v", err) - } - - if spec, ok := resolvePullSpec(&stableImageStream, tag.Name, false); ok { - s.params.Set(componentToParamName(tag.Name), spec) - } - } + for _, tag := range is.Spec.Tags { + spec, ok := resolvePullSpec(is, tag.Name, false) + if !ok { + continue } + s.params.Set(componentToParamName(tag.Name), spec) } return nil @@ -322,12 +234,7 @@ func (s *releaseImagesTagStep) imageFormat() (string, error) { return "REGISTRY", err } registry := strings.SplitN(spec, "/", 2)[0] - var format string - if len(s.config.Name) > 0 { - format = fmt.Sprintf("%s/%s/%s:%s", registry, s.jobSpec.Namespace, fmt.Sprintf("%s%s", s.config.NamePrefix, api.StableImageStream), api.ComponentFormatReplacement) - } else { - format = fmt.Sprintf("%s/%s/%s:%s", registry, s.jobSpec.Namespace, fmt.Sprintf("%s%s", s.config.NamePrefix, api.ComponentFormatReplacement), s.config.Tag) - } + format := fmt.Sprintf("%s/%s/%s:%s", registry, s.jobSpec.Namespace, fmt.Sprintf("%s%s", s.config.NamePrefix, api.StableImageStream), api.ComponentFormatReplacement) return format, nil }