Skip to content

Conversation

@jottofar
Copy link
Contributor

@jottofar jottofar commented Jul 22, 2020

Changes to verify package in preparation for move to library-go

The CVO verify package will be moved to library-go for reuse by 'oc adm release mirror' and CVO. Changes are being made here in CVO before the move to provide a cleaner move to library-go.

Logic to create verifier was broken out into a new configmap method NewFromManifests. This method uses k8s encoding and as a result existing test cases had to be changed from creating manifests from from declarations within the test cases itself to creating them from config map files. In this manner the lib/Manifest object is properly created with its Raw bytes field populated.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 22, 2020
@jottofar jottofar force-pushed the ota-190-verify-reuse-prep branch from 5214063 to e792735 Compare July 22, 2020 15:54
@jottofar
Copy link
Contributor Author

/assign @wking

@jottofar jottofar force-pushed the ota-190-verify-reuse-prep branch from e792735 to 2bde02f Compare July 22, 2020 19:53
@jottofar jottofar force-pushed the ota-190-verify-reuse-prep branch from 2bde02f to f16cb2d Compare July 24, 2020 14:01
@jottofar
Copy link
Contributor Author

@wking I went back to my original approach which adds a method LoadConfigMapVerifierDataFromManifest, to be used by oc, and allows the prefix constants to remain internal. Note that I added k8s decode/encode to util which is used by new method.

@jottofar
Copy link
Contributor Author

/retest

@jottofar jottofar force-pushed the ota-190-verify-reuse-prep branch from dd8df78 to eeb33e9 Compare July 30, 2020 13:09
@jottofar
Copy link
Contributor Author

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 30, 2020
@jottofar jottofar force-pushed the ota-190-verify-reuse-prep branch 4 times, most recently from bb23989 to f929d03 Compare July 31, 2020 21:55
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 31, 2020
@jottofar
Copy link
Contributor Author

/unhold

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 31, 2020
@jottofar jottofar force-pushed the ota-190-verify-reuse-prep branch from f929d03 to 41c79e9 Compare July 31, 2020 22:01
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 31, 2020
@jottofar
Copy link
Contributor Author

jottofar commented Aug 3, 2020

/retest

@jottofar jottofar force-pushed the ota-190-verify-reuse-prep branch from b39b35f to 33b8eb8 Compare August 9, 2020 16:13
@jottofar
Copy link
Contributor Author

/test unit

@jottofar
Copy link
Contributor Author

/retest

1 similar comment
@jottofar
Copy link
Contributor Author

/retest

@jottofar jottofar force-pushed the ota-190-verify-reuse-prep branch from 33b8eb8 to d1d12ab Compare August 11, 2020 22:10
@jottofar
Copy link
Contributor Author

/retest

Copy link

@vrutkovs vrutkovs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

The CVO verify package will be moved to library-go for reuse by 'oc adm
release mirror' and CVO. Changes are being made here in CVO before the
move to provide a cleaner move to library-go.

Logic to create verifier was broken out into a new configmap method
NewFromManifests. This method uses k8s encoding and as a result
existing test cases had to be changed from creating manifests from
from declarations within the test cases itself to creating them from
config map files. In this manner the lib/Manifest object is properly
created with its Raw bytes field populated.
@jottofar jottofar force-pushed the ota-190-verify-reuse-prep branch from d1d12ab to 472cc1b Compare August 17, 2020 12:40
@jottofar
Copy link
Contributor Author

/retest

1 similar comment
@jottofar
Copy link
Contributor Author

/retest

@jottofar
Copy link
Contributor Author

/test unit

@wking
Copy link
Member

wking commented Aug 18, 2020

One unit test I don't understand, otherwise looks good to me. Pull the hold if I'm just confused, or if you want to punt a fix to follow-up work.

/lgtm
/hold

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Aug 18, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jottofar, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jottofar
Copy link
Contributor Author

/unhold

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 18, 2020
@openshift-merge-robot openshift-merge-robot merged commit 247ab9a into openshift:master Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants