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.

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

enable registry middleware acceptschema2 #13428

Merged
merged 2 commits into from
Jun 16, 2017

Conversation

aweiteka
Copy link
Contributor

Signed-off-by: Aaron Weitekamp aweiteka@redhat.com

Signed-off-by: Aaron Weitekamp <aweiteka@redhat.com>
@ncdc
Copy link
Contributor

ncdc commented Mar 17, 2017

@ncdc
Copy link
Contributor

ncdc commented Mar 17, 2017

cc @vbatts

@smarterclayton
Copy link
Contributor

Also need to double check Ansible.

@mfojtik
Copy link
Contributor

mfojtik commented Mar 20, 2017

@miminar @legionus PTAL

@legionus
Copy link
Contributor

Yep. Also need to change the default in the middleware code and in the ansible

Signed-off-by: Aaron Weitekamp <aweiteka@redhat.com>
@openshift-bot
Copy link
Contributor

Evaluated for origin testextended up to 304fba4

@miminar miminar mentioned this pull request Mar 21, 2017
@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/testextended FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended/65/) (Base Commit: 010f390) (Extended Tests: core(imageapis), core(builds))

@vbatts
Copy link

vbatts commented Mar 21, 2017

@miminar (carrying forward the conversation of #8938 (comment))

Does "accepting" schema2 mean "forcing" schema2? It does not sound like it would.

@miminar
Copy link

miminar commented Mar 21, 2017

Does "accepting" schema2 mean "forcing" schema2? It does not sound like it would.

@vbatts That's correct. It's up to the client, what version of schema it will upload. Setting the acceptschema2 to true only allows for the schema to be accepted - docker (1.10+ will not fall back to schema 1). Older clients or rh docker daemon running with a flag to enforce schema 1 will still push schema 1 regardless of this option.

Do we need acceptschema1 as well? 😄

@miminar
Copy link

miminar commented Mar 21, 2017

Apparently, there cannot be two different extended focus statements. Trying one more time with a regexp

@vbatts
Copy link

vbatts commented Mar 21, 2017

@miminar i would honestly see if we can just jump to allowing v2.s2, skipping v2.s1, and hopefully next will be allowing oci-v1. To address your concern, how worried are we, that a newer client will push v2.s2 content, and then an older client is broken in fetching it?

@mtrmac thoughts on just allowing v2.s2 format and skipping v2.s1 on the openshift registry?

@ncdc
Copy link
Contributor

ncdc commented Mar 21, 2017

AFAIK we require docker 1.10, if not 1.12. cc @smarterclayton @sdodson @brenton @eparis to confirm.

@mtrmac
Copy link
Contributor

mtrmac commented Mar 21, 2017

I would very much like acceptschema2 to be true by default. I don’t see anything about “skipping schema1” in this PR.

@brenton
Copy link
Contributor

brenton commented Mar 21, 2017

@ncdc Here's the docker support matrix for ocp:

3.0, 3.1 = docker 1.8
3.2, 3.3 = docker 1.10
3.4 = docker 1.12

@ncdc
Copy link
Contributor

ncdc commented Mar 21, 2017

Thanks @brenton. Given that info, I don't see any need to support schema 1 in our registry any more. Anyone disagree?

@brenton
Copy link
Contributor

brenton commented Mar 21, 2017

I'm pretty sure images from registry.access.redhat.com use v2.s1. Would that no longer supporting this schema cause breakage?

@ncdc
Copy link
Contributor

ncdc commented Mar 21, 2017

I don't think we're talking about removing support for v2.s1. We're just talking about allowing v2.s2. Right?

@smarterclayton
Copy link
Contributor

What's the status on his?

@aweiteka
Copy link
Contributor Author

Based on the comments this is safe to merge.

@aweiteka aweiteka self-assigned this Apr 17, 2017
@aweiteka
Copy link
Contributor Author

@mfojtik I recommend pulling this into the next release. Sooner than later would be good to have some time with origin testing.

@ncdc
Copy link
Contributor

ncdc commented Apr 17, 2017

It'll have to wait until next sprint starts (4/24)

@aweiteka
Copy link
Contributor Author

aweiteka commented May 9, 2017

@mfojtik ping

@mfojtik
Copy link
Contributor

mfojtik commented May 9, 2017

@legionus @miminar ?

@miminar
Copy link

miminar commented May 10, 2017

I don't think we're talking about removing support for v2.s1. We're just talking about allowing v2.s2. Right?

That's right. To be precise, this concerns just the manifests created via a push to the integrated registry. We already allow for consuming v2.s2 either by taging or image import.

@aweiteka can you please change false to true here as well?

Hmm, looking at this I believe there's a bit more work todo in our tests. @aweiteka do you perhaps want me to take over?

[testextended][extended:core(builds|imageapis|images)]

@aweiteka
Copy link
Contributor Author

Hmm, looking at this I believe there's a bit more work todo in our tests. @aweiteka do you perhaps want me to take over?

@miminar that would be great, if you don't mind. :)

@smarterclayton smarterclayton added this to the 3.6.0 milestone Jun 4, 2017
@smarterclayton
Copy link
Contributor

This is required for 3.6

@smarterclayton
Copy link
Contributor

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 304fba4

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1924/) (Base Commit: 90592c5)

@smarterclayton
Copy link
Contributor

@dmage do you think you can help get this over the hump since @miminar has a billion things going on?

@dmage
Copy link
Contributor

dmage commented Jun 5, 2017

I'll take a look as soon as I've finished with registry pruning.

re[testextended][extended:core(builds|imageapis|images)]

@openshift-bot
Copy link
Contributor

The Origin testextended job could not be run again for this pull request.

  • If the proposed changes in this pull request caused the job to fail, update this pull request with new code to fix the issue(s).
  • If flaky tests caused the job to fail, leave a comment with links to the GitHub issue(s) in the openshift/origin repository with the kind/test-flake label that are tracking the flakes. If no issue already exists for the flake you encountered, create one.
  • If something else like CI system downtime or maintenance caused the job to fail, contact a member of the Team Project Committers group to trigger the job again.

Copy link
Contributor

@legionus legionus left a comment

Choose a reason for hiding this comment

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

LGTM

@smarterclayton
Copy link
Contributor

[merge][severity:blocker]

Unless there's a good reason still?

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 304fba4

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge FAILURE (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/1014/) (Base Commit: ee15d99) (PR Branch Commit: 304fba4) (Extended Tests: blocker)

@smarterclayton
Copy link
Contributor

Daemmonset flake, force merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet