Default stable image stream from RELEASE_IMAGE_LATEST
#287
Conversation
openshift-ci-robot
added
approved
smarterclayton
force-pushed
the
init_initial
branch
from
0ccec44 to
9dd57be
Compare
| } | ||
|
|
||
| if size > 0 { | ||
| if size > 1*1000*1000 { |
There was a problem hiding this comment.
Reduce pointless output. Will comment. The only reason to print it is when we upload lots of content and the job just sits there.
| func (s *podStep) Run(ctx context.Context, dry bool) error { | ||
| log.Printf("Executing %s %s", s.name, s.config.As) | ||
|
|
||
| if !s.config.SkipLogs { |
There was a problem hiding this comment.
This smells bad, why isn't this going to make for inscrutable logs that admins hate?
There was a problem hiding this comment.
There are pod steps called within jobs (where you want artifacts). The log output when you call these nested logs is more confusing - the step for "build release image" doesn't need to show executing pod release-latest because it already said what it is doing. We still print pod logs when they fail which is 95% of all debugging.
There was a problem hiding this comment.
The import from initial needs to run two pods, but the pods should never fail and the output of the nested pod within the import step is very confusing when reading it. Only the top most step matters in terms of you needing to understand what is happening.
There was a problem hiding this comment.
Added a long godoc on PodStepConfiguration to explain this, let me know if that helps clarify.
| // importFromReleaseImage uses the provided release image and updates the stable / release streams as | ||
| // appropriate with the contents of the payload so that downstream components are using the correct images. | ||
| // The most common case is to use the correct installer image, tests, and cli commands. | ||
| func (s *assembleReleaseStep) importFromReleaseImage(ctx context.Context, dry bool, providedImage string) error { |
There was a problem hiding this comment.
I don't think I fully understand why this is necessary, is this lacking in current version of ci-op or is it just when you want to run a job and specify a specific release?
There was a problem hiding this comment.
We want to run a ci-operator job where the input to the job is a release image (release qualification tests) or we want to run an upgrade job where we take a previously released image (upgrade tests) where we won't have an image stream to represent them. The upgrade job:
Test the upgrade from 4.0.3 to a PR in 4.1 master against the cluster-version-operator component
wants to use the installer image from 4.0.3, so we need to populate stable-initial with the contents of 4.0.3 while stable continues to be populated from 4.1 master.
There was a problem hiding this comment.
The other use case is:
Test installing 4.1.4 and run e2e tests against it
We don't want to use the image stream for 4.1.4, we want to grab the payload for 4.1.4 and then use the installer image out of the 4.1.4 payload to install the cluster and the tests image out of the 4.1.4 payload (in this case) to run the e2e tests.
There was a problem hiding this comment.
I added a super long godoc to the type, hope that clarifies.
smarterclayton
force-pushed
the
init_initial
branch
from
9dd57be to
0ea910b
Compare
Periodic jobs that want to run a PR from a known `RELEASE_IMAGE_LATEST` need access to the `cli` and `installer` images out of the payload. This fixes problems we have testing older installers and cli binaries against newer payloads and fixes the current break in the release controllers. This is also required for properly testing the installer. Use standard oc commands (that are API forward compatible) to extract the `cli` image and the `installer` image from the payload and set them on stable if RELEASE_IMAGE_LATEST is provided as an input (as they are for the release blocking jobs) or on stable-initial if RELEASE_IMAGE_INITIAL is provided. Hide nested container output because it is confusing when leveraging a pod from within an existing step.
smarterclayton
force-pushed
the
init_initial
branch
from
0ea910b to
16f104c
Compare
|
Added requested comments, tried to clarify intent better. |
The release jobs set IMAGE_FORMAT to bypass creating images, and when a job requests RELEASE_IMAGE_INITIAL but doesn't set it to to something we should skip creating the initial image.
smarterclayton
force-pushed
the
init_initial
branch
from
e1156c0 to
2cfaa7c
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: smarterclayton, stevekuznetsov The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 participants
Periodic jobs that want to run a PR from a known
RELEASE_IMAGE_LATESTneed access to the
cliandinstallerimages out of the payload. Thisfixes problems we have testing older installers and cli binaries against
newer payloads and fixes the current break in the release controllers. This
is also required for properly testing the installer.
Use standard oc commands (that are API forward compatible) to extract the
cliimage and theinstallerimage from the payload and set them onstable if RELEASE_IMAGE_LATEST is provided as an input (as they are for the
release blocking jobs) or on stable-initial if RELEASE_IMAGE_INITIAL is
provided.
Hide nested container output because it is confusing when leveraging a pod
from within an existing step.
Builds on #284