Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

IR-52: docker/distribution v2.7.1 #252

Merged
merged 2 commits into from Nov 2, 2020
Merged

IR-52: docker/distribution v2.7.1 #252

merged 2 commits into from Nov 2, 2020

Conversation

dmage
Copy link
Member

@dmage dmage commented Oct 12, 2020

The image registry used some version of docker/distribution from master. This pull request updates docker/distribution to the release version 2.7.1.

Upstream changes:

None of them are available/configurable in OCP.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 12, 2020
@dmage
Copy link
Member Author

dmage commented Oct 12, 2020

/hold

This PR shouldn't be merged until it's signed off by QE.

@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 Oct 12, 2020
@dmage
Copy link
Member Author

dmage commented Oct 12, 2020

/label qe-approved

@openshift-ci-robot openshift-ci-robot added the qe-approved Signifies that QE has signed off on this PR label Oct 12, 2020
@dmage
Copy link
Member Author

dmage commented Oct 12, 2020

/remove-label qe-approved

@openshift-ci-robot openshift-ci-robot removed the qe-approved Signifies that QE has signed off on this PR label Oct 12, 2020
@dmage
Copy link
Member Author

dmage commented Oct 12, 2020

/retest

@dmage
Copy link
Member Author

dmage commented Oct 12, 2020

/assign @ricardomaraschini

@dmage
Copy link
Member Author

dmage commented Oct 14, 2020

go.sum conflict is fixed.

dmage
dmage commented Oct 19, 2020
View reviewed changes
go.mod Outdated
k8s.io/klog/v2 v2.3.0
)

replace (
github.com/docker/distribution => github.com/openshift/docker-distribution v0.0.0-20200915090847-1d6fb9bf35ab
github.com/docker/distribution => github.com/dmage/distribution v0.0.0-20201012083032-63d38f155de2
Copy link
Member Author

Choose a reason for hiding this comment

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

This will be replaced to github.com/openshift/docker-distribution after all approvals are there.

ricardomaraschini
ricardomaraschini approved these changes Oct 19, 2020
View reviewed changes
Copy link
Contributor

@ricardomaraschini ricardomaraschini left a comment

Choose a reason for hiding this comment

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

/lgtm

pkg/dockerregistry/server/auth.go
@@ -110,7 +110,7 @@ func (ac *authChallenge) Error() string {
}

// SetHeaders sets the basic challenge header on the response.
func (ac *authChallenge) SetHeaders(w http.ResponseWriter) {
func (ac *authChallenge) SetHeaders(req *http.Request, w http.ResponseWriter) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This was made because an interface somewhere else has changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, see vendor/github.com/docker/distribution/registry/auth/auth.go.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2020
@dmage
Copy link
Member Author

dmage commented Oct 19, 2020

/retest

1 similar comment
@dmage
Copy link
Member Author

dmage commented Oct 19, 2020

/retest

@dmage
Copy link
Member Author

dmage commented Oct 19, 2020

/assign @wzheng1 @bmcelvee @sferich888

This PR got approval from the engineering team and is ready for next approvals. Please use /label XXX-approved (where XXX is qe, docs, or px) to approve this PR.

@wzheng1 - no new functionality is added there, so you may want to make regression testing.
@bmcelvee @sferich888 - no new functionality, should I ping you on such PRs so that you can ack no-doc/no-feature?

@wzheng1
Copy link

wzheng1 commented Oct 20, 2020

@dmage I have tested this pr, but the cluster I started up via cluster-bot is with version 4.6.0-0.ci.test-2020-10-20-063531-ci-ln-6g1cc4t, is it fine?

@dmage
Copy link
Member Author

dmage commented Oct 20, 2020

@wzheng1 it's a temporary version, I don't know if there is a way to know what PRs it contains. If you started your cluster using launch openshift/image-registry#252, then it is fine.

@wzheng1
Copy link

wzheng1 commented Oct 20, 2020

@dmage OK, get it, thanks! I launched a cluster with command: launch #252, and a cluster with version 4.6.0-0.ci.test-2020-10-20-063531-ci-ln-6g1cc4t is running for my test. Regression test with image registry is finished with no issue, so adding below label now:
/label qe-approved

@openshift-ci-robot openshift-ci-robot added the qe-approved Signifies that QE has signed off on this PR label Oct 20, 2020
@bmcelvee
Copy link

@dmage good question, if you think we should add no-doc items like this one to the 4.7 release notes, it couldn't hurt. Otherwise, it's probably not necessary. Regardless, I think this one is good.

/label docs-approved

@openshift-ci-robot openshift-ci-robot added the docs-approved Signifies that Docs has signed off on this PR label Oct 20, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

19 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@dmage
Copy link
Member Author

dmage commented Oct 26, 2020

/hold
Something wrong with our CI.

@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 Oct 26, 2020
@dmage
Copy link
Member Author

dmage commented Oct 26, 2020

/test e2e-aws

@dmage
Copy link
Member Author

dmage commented Nov 2, 2020

/retest

@dmage
Copy link
Member Author

dmage commented Nov 2, 2020

/hold cancel

@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 Nov 2, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit f47575c into openshift:master Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ricardomaraschini ricardomaraschini ricardomaraschini approved these changes

@adambkaplan adambkaplan Awaiting requested review from adambkaplan

Assignees

@ricardomaraschini ricardomaraschini

@wzheng1 wzheng1

@bmcelvee bmcelvee

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. docs-approved Signifies that Docs has signed off on this PR lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@dmage @wzheng1 @bmcelvee @sferich888 @ricardomaraschini @openshift-ci-robot @openshift-bot @openshift-merge-robot