Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Feb 20, 2020

To make it easier to understand where things are breaking down when they don't work as expected.

To make it easier to understand where things are breaking down when
they don't work as expected [1].

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1782982#c16
@openshift-ci-robot openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 20, 2020
@openshift-ci-robot
Copy link
Contributor

@wking: This pull request references Bugzilla bug 1805172, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Bug 1805172: pkg/verify/verifyconfigmap: Add klog logging

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 20, 2020
@wking
Copy link
Member Author

wking commented Feb 20, 2020

upgrade:

fail [github.com/openshift/origin/test/extended/util/disruption/controlplane/controlplane.go:56]: Feb 20 15:20:47.085: API was unreachable during upgrade for at least 4m44s:

More on that in rhbz#1801885 and rhbz#1802246.

/retest

@wking
Copy link
Member Author

wking commented Feb 20, 2020

Not much in the CI logs, but we'd need a rollback test (which we don't have at the moment) to really excercise the verify code.

$ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-version-operator/328/pull-ci-openshift-cluster-version-operator-master-e2e-aws-upgrade/515/artifacts/e2e-aws-upgrade/must-gather/registry-svc-ci-openshift-org-ci-op-vdz4rwcq-stable-initial-sha256-ee4eae4c297a6f0c80de95d12266c61f7348349a3e72d909a294644e8371e3aa/namespaces/openshift-cluster-version/pods/cluster-version-operator-5c45987b67-5w76s/cluster-version-operator/cluster-version-operator/logs/current.log | grep signature
2020-02-20T15:12:51.788130224Z I0220 15:12:51.788071       1 cvo.go:264] Verifying release authenticity: All release image digests must have GPG signatures from verifier-public-key-openshift-ci (D04761B116203B0C0859B61628B76E05B923888E: openshift-ci) - will check for signatures in containers/image format at https://storage.googleapis.com/openshift-release/test-1/signatures/openshift/release and from config maps in openshift-config-managed with label "release.openshift.io/verification-signatures"
$ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-version-operator/328/pull-ci-openshift-cluster-version-operator-master-e2e-aws/1131/artifacts/e2e-aws/must-gather/registry-svc-ci-openshift-org-ci-op-vdz4rwcq-stable-sha256-ee4eae4c297a6f0c80de95d12266c61f7348349a3e72d909a294644e8371e3aa/namespaces/openshift-cluster-version/pods/cluster-version-operator-d4fd4685-mqhdt/cluster-version-operator/cluster-version-operator/logs/current.log | grep signature
2020-02-20T13:55:22.54267942Z I0220 13:55:22.542613       1 cvo.go:264] Verifying release authenticity: All release image digests must have GPG signatures from verifier-public-key-openshift-ci (D04761B116203B0C0859B61628B76E05B923888E: openshift-ci) - will check for signatures in containers/image format at https://storage.googleapis.com/openshift-release/test-1/signatures/openshift/release and from config maps in openshift-config-managed with label "release.openshift.io/verification-signatures"
2020-02-20T14:33:14.784372593Z I0220 14:33:14.784327       1 request.go:538] Throttling request took 122.530496ms, request: GET:https://127.0.0.1:6443/api/v1/namespaces/openshift-config-managed/configmaps/signatures-managed
2020-02-20T14:33:14.934402669Z I0220 14:33:14.934342       1 request.go:538] Throttling request took 146.630952ms, request: PUT:https://127.0.0.1:6443/api/v1/namespaces/openshift-config-managed/configmaps/signatures-managed

Presence of throttling there suggests that maybe some of my new logs are not firing. Not sure why they wouldn't, we do have V(4) stuff in the logs...

@sdodson
Copy link
Member

sdodson commented Feb 25, 2020

/cc @jottofar @vrutkovs PTAL
/uncc @smarterclayton

@openshift-ci-robot openshift-ci-robot requested review from vrutkovs and removed request for smarterclayton February 25, 2020 16:16
@openshift-ci-robot
Copy link
Contributor

@sdodson: GitHub didn't allow me to request PR reviews from the following users: jottofar, PTAL.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @jottofar @vrutkovs PTAL
/uncc @smarterclayton

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sdodson
Copy link
Member

sdodson commented Feb 25, 2020

/cc @jottofar @vrutkovs

@openshift-ci-robot
Copy link
Contributor

@sdodson: GitHub didn't allow me to request PR reviews from the following users: jottofar.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @jottofar @vrutkovs

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

klog.V(4).Infof("searching for %s in signature config map %s", prefix, cm.ObjectMeta.Name)
for k, v := range cm.BinaryData {
if strings.HasPrefix(k, prefix) {
klog.V(4).Infof("key %s from signature config map %s matches %s", k, cm.ObjectMeta.Name, digest)

Choose a reason for hiding this comment

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

Log prefix here in case it gets incorrectly generated? A better idea would be logging result in digestToKeyPrefix too

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't get here if the digest string had incorrect format. Logging prefix is just for info.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm already logging the prefix a few lines up with searching for {prefix} in signature config map...?

@sdodson
Copy link
Member

sdodson commented Feb 25, 2020

/cc @jottofar

@jottofar
Copy link
Contributor

/test e2e-aws-upgrade

@jottofar
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 26, 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

@openshift-merge-robot openshift-merge-robot merged commit 5e684d5 into openshift:master Feb 26, 2020
@openshift-ci-robot
Copy link
Contributor

@wking: All pull requests linked via external trackers have merged. Bugzilla bug 1805172 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1805172: pkg/verify/verifyconfigmap: Add klog logging

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sdodson
Copy link
Member

sdodson commented Feb 26, 2020

/cherry-pick release-4.4

@openshift-cherrypick-robot

@sdodson: new pull request created: #332

Details

In response to this:

/cherry-pick release-4.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@wking wking deleted the config-map-signature-logging branch April 15, 2020 04:38
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. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants