Skip to content

Conversation

@ecordell
Copy link
Contributor

Bumps operator-registry, which fixes two BZs:

https://bugzilla.redhat.com/show_bug.cgi?id=1790785
https://bugzilla.redhat.com/show_bug.cgi?id=1772942 (already fixed in master, but this needs to propagate to 4.3)

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 21, 2020
@ecordell ecordell changed the title bump(operator-framework/operator-registry) Bug 1790785: bump(operator-framework/operator-registry) Feb 21, 2020
@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Feb 21, 2020
@openshift-ci-robot
Copy link

@ecordell: This pull request references Bugzilla bug 1790785, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Bug 1790785: bump(operator-framework/operator-registry)

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
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ecordell
To complete the pull request process, please assign smarterclayton
You can assign the PR to them by writing /assign @smarterclayton in a comment when ready.

The full list of commands accepted by this bot can be found 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

@ecordell ecordell force-pushed the operator-registry-1.5.10 branch from d657b40 to f447321 Compare February 21, 2020 23:33
@ecordell
Copy link
Contributor Author

/retest

1 similar comment
@ecordell
Copy link
Contributor Author

/retest

github.com/mtrmac/gpgme v0.1.2 // indirect
github.com/opencontainers/go-digest v1.0.0-rc1
github.com/openshift/api v0.0.0-20200205133042-34f0ec8dab87
github.com/openshift/api v3.9.1-0.20190924102528-32369d4db2ad+incompatible
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unexpected change.

Copy link
Contributor

Choose a reason for hiding this comment

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

From what I checked this is somehow a transitive dependency that's coming only when I change operator-registry, if I bump openshift deps this will work just fine picking newer version, which makes me worried a lot about that bump.

k8s.io/apiserver v0.17.1
k8s.io/cli-runtime v0.17.0
k8s.io/client-go v0.17.1
k8s.io/client-go v8.0.0+incompatible
Copy link
Contributor

Choose a reason for hiding this comment

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

Same this one.

Choose a reason for hiding this comment

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

I do want to point out that this is replaced later at

oc/go.mod

Line 77 in f447321

k8s.io/client-go => github.com/openshift/kubernetes-client-go v0.0.0-20191211181558-5dcabadb2b45
. Therefore it does not matter that is it changed at this stage.

k8s.io/klog v1.0.0
k8s.io/kubectl v0.0.0
k8s.io/kubernetes v0.0.0-00010101000000-000000000000
k8s.io/kubernetes v1.16.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

Choose a reason for hiding this comment

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

Same as above at

oc/go.mod

Line 90 in f447321

k8s.io/kubernetes => github.com/openshift/kubernetes v1.17.0-alpha.0.0.20191216151305-079984b0a154
.

replace (
github.com/apcera/gssapi => github.com/openshift/gssapi v0.0.0-20161010215902-5fb4217df13b
github.com/containers/image => github.com/openshift/containers-image v0.0.0-20190130162819-76de87591e9d
github.com/openshift/api => github.com/openshift/api v0.0.0-20200205133042-34f0ec8dab87
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not needed.

Choose a reason for hiding this comment

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

This replaces https://github.com/openshift/oc/pull/316/files#r383280617, therefore, it does not change dependencies at all.

Copy link

@Bowenislandsong Bowenislandsong left a comment

Choose a reason for hiding this comment

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

Thank you very much for the review. I did try very hard to sort everything out. However, It does seem like these changes are not affecting the dependencies at all since they are overwritten by replace right after. Other than the fact that they do not look as pretty in format. These changes should be harmless. Please let me know what you think. Thank you.

k8s.io/apiserver v0.17.1
k8s.io/cli-runtime v0.17.0
k8s.io/client-go v0.17.1
k8s.io/client-go v8.0.0+incompatible

Choose a reason for hiding this comment

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

I do want to point out that this is replaced later at

oc/go.mod

Line 77 in f447321

k8s.io/client-go => github.com/openshift/kubernetes-client-go v0.0.0-20191211181558-5dcabadb2b45
. Therefore it does not matter that is it changed at this stage.

k8s.io/klog v1.0.0
k8s.io/kubectl v0.0.0
k8s.io/kubernetes v0.0.0-00010101000000-000000000000
k8s.io/kubernetes v1.16.0

Choose a reason for hiding this comment

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

Same as above at

oc/go.mod

Line 90 in f447321

k8s.io/kubernetes => github.com/openshift/kubernetes v1.17.0-alpha.0.0.20191216151305-079984b0a154
.

replace (
github.com/apcera/gssapi => github.com/openshift/gssapi v0.0.0-20161010215902-5fb4217df13b
github.com/containers/image => github.com/openshift/containers-image v0.0.0-20190130162819-76de87591e9d
github.com/openshift/api => github.com/openshift/api v0.0.0-20200205133042-34f0ec8dab87

Choose a reason for hiding this comment

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

This replaces https://github.com/openshift/oc/pull/316/files#r383280617, therefore, it does not change dependencies at all.

github.com/mtrmac/gpgme v0.1.2 // indirect
github.com/opencontainers/go-digest v1.0.0-rc1
github.com/openshift/api v0.0.0-20200205133042-34f0ec8dab87
github.com/openshift/api v3.9.1-0.20190924102528-32369d4db2ad+incompatible
Copy link
Contributor

Choose a reason for hiding this comment

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

From what I checked this is somehow a transitive dependency that's coming only when I change operator-registry, if I bump openshift deps this will work just fine picking newer version, which makes me worried a lot about that bump.

@soltysh
Copy link
Contributor

soltysh commented Feb 26, 2020

/hold
I'll have another look at it

@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 Feb 26, 2020
@soltysh
Copy link
Contributor

soltysh commented Feb 27, 2020

I'll keep this on hold, I have agreement with @ecordell he'll look into fixing some of deps in operator-registry.

@Bowenislandsong
Copy link

I want to mention that this solution makes the least amount of effort in changes and it is easy to backport.

Bowenislandsong pushed a commit to Bowenislandsong/oc that referenced this pull request Feb 27, 2020
Bumps operator-registry, which fixes two BZs:

https://bugzilla.redhat.com/show_bug.cgi?id=1790785
https://bugzilla.redhat.com/show_bug.cgi?id=1772942 (already fixed in master, but this needs to propagate to 4.3)

This commit supersedes openshift#316.
@Bowenislandsong
Copy link

Bowenislandsong commented Feb 27, 2020

/close
superseded by #329

@soltysh soltysh closed this Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants