start: Initialize the payload and fail if it can't be read at startup #169
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci-robot
added
approved
smarterclayton
force-pushed
the
sign
branch
2 times, most recently
from
9be341c
to
635282b
Compare
openshift-ci-robot
added
size/M
|
/retest |
| func (optr *Operator) InitializeFromPayload() error { | ||
| update, err := payload.LoadUpdate(optr.defaultPayloadDir(), optr.releaseImage) | ||
| if err != nil { | ||
| return fmt.Errorf("the local release contents are invalid - no current version can be determined from disk: %v", err) |
There was a problem hiding this comment.
nit can we use already imported github.com/pkg/errors for wrapping the error?
return errors.Wrap(err, "failed to release image - no current version can be determined from disk")
| return fmt.Errorf("the local release contents are invalid - no current version can be determined from disk: %v", err) | ||
| } | ||
| // XXX: set this to the cincinnati version in preference | ||
| if _, err := semver.Parse(update.ImageRef.Name); err != nil { |
There was a problem hiding this comment.
are we adding requirement for all release images must be semver ?
|
Can you point include in the commit message why we are moving to requirement that all release image versions must be semver? this is a requirement for CVO will Cincinnati support this constraint? |
Avoid the possibility of a bad release image being created that can't correctly read the payload by failing hard on startup if the payload is invalid. Both cincinnati and the release creation process require semantic versions. The release controller will fail jobs that are not semver.
|
Cincinnati already enforces this, as does release creation. I just
realized in here that we don't enforce.
…On Mon, Apr 22, 2019 at 5:32 PM Abhinav Dahiya ***@***.***> wrote:
Can you point include in the commit message why we are moving to
requirement that all release image versions must be semver? this is a
requirement for CVO will Cincinnati support this constraint?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#169 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAI37J4ZBFZZ6TEBVJAFXPLPRYVFXANCNFSM4HHIVOOQ>
.
|
|
Updated commit message
On Mon, Apr 22, 2019 at 10:41 PM Clayton Coleman <ccoleman@redhat.com>
wrote:
… Cincinnati already enforces this, as does release creation. I just
realized in here that we don't enforce.
On Mon, Apr 22, 2019 at 5:32 PM Abhinav Dahiya ***@***.***>
wrote:
> Can you point include in the commit message why we are moving to
> requirement that all release image versions must be semver? this is a
> requirement for CVO will Cincinnati support this constraint?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#169 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAI37J4ZBFZZ6TEBVJAFXPLPRYVFXANCNFSM4HHIVOOQ>
> .
>
|
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, smarterclayton The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Avoid the possibility of a bad release image being created that can't
correctly read the payload by failing hard on startup if the payload is
invalid.