Skip to content

pkg/asset/releaseimage/pullspec: Include override pullspec in log message#6460

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
wking:log-override-pullspec
Oct 5, 2022
Merged

pkg/asset/releaseimage/pullspec: Include override pullspec in log message#6460
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
wking:log-override-pullspec

Conversation

@wking
Copy link
Copy Markdown
Member

@wking wking commented Oct 4, 2022

This code-branch should be very rare, probably mostly internal testing. A chattier line in those situations isn't a big user-experience concern, and including the pullspec makes it easier for folks who are trying to understand internal-test results to figure out which pullspec was being used.

The log line is originally from 456a373 (#1910), and it was tweaked in cd0b70f (#2361), and neither of those make explicit arguments for why they didn't include the pullspec.

…sage

This code-branch should be very rare, probably mostly internal
testing.  A chattier line in those situations isn't a big
user-experience concern, and including the pullspec makes it easier
for folks who are trying to understand internal-test results to figure
out which pullspec was being used.

The log line is originally from 456a373 (pkg: Use the sources from
InstalConfig to create containers-registries.conf for bootstap,
2019-06-28, openshift#1910), and it was tweaked in cd0b70f (asset: Make
warning message read for humans, 2019-09-15, openshift#2361), and neither of
those make explicit arguments for why they didn't include the
pullspec.
@Prashanth684
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 4, 2022
@jhixson74
Copy link
Copy Markdown
Member

/approve

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 4, 2022
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 84aa822 and 2 for PR HEAD 3436ea8 in total

@patrickdillon
Copy link
Copy Markdown
Contributor

/approve

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Oct 4, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhixson74, patrickdillon

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:
  • OWNERS [jhixson74,patrickdillon]

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

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD f386e5d and 1 for PR HEAD 3436ea8 in total

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Oct 5, 2022

@wking: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-openstack 3436ea8 link false /test e2e-openstack
ci/prow/e2e-libvirt 3436ea8 link false /test e2e-libvirt
ci/prow/okd-e2e-aws-ovn 3436ea8 link false /test okd-e2e-aws-ovn
ci/prow/okd-e2e-aws-upgrade 3436ea8 link false /test okd-e2e-aws-upgrade
ci/prow/e2e-ibmcloud-ovn 3436ea8 link false /test e2e-ibmcloud-ovn

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit 3379601 into openshift:master Oct 5, 2022
@wking wking deleted the log-override-pullspec branch October 5, 2022 15:46
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.

6 participants