OCPCLOUD-2010: Re-vendor api and library-go for external platform support by adriengentil · Pull Request #736 · openshift/cluster-kube-controller-manager-operator

adriengentil · 2023-05-15T16:02:40Z

Revendor openshift/api and openshift/library-go in order to support the
external platform type.

library-go PR: openshift/library-go#1496
API PR: openshift/api#1434

openshift-ci-robot · 2023-05-15T16:02:43Z

@adriengentil: This pull request references OCPCLOUD-2010 which is a valid jira issue.

Details

In response to this:

Revendor openshift/api and openshift/library-go in order to support the
external platform type.

library-go PR: openshift/library-go#1496
API PR: openshift/api#1434

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 · 2023-05-15T16:02:57Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

adriengentil · 2023-05-16T08:13:16Z

/test ?

openshift-ci · 2023-05-16T08:13:20Z

@adriengentil: The following commands are available to trigger required jobs:

/test e2e-aws-operator
/test e2e-aws-operator-preferred-host
/test e2e-aws-ovn
/test e2e-aws-ovn-upgrade
/test images
/test unit
/test verify
/test verify-deps

Use /test all to run all jobs.

Details

In response to this:

/test ?

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.

adriengentil · 2023-05-16T08:13:34Z

/test unit

elmiko

lgtm, just waiting on the upstream PRs so we can revendor

elmiko

just a question, otherwise lgtm

elmiko · 2023-06-15T08:21:36Z

go.mod

 	sigs.k8s.io/yaml v1.3.0 // indirect
 )
+
+replace vbom.ml/util => github.com/fvbommel/util v0.0.0-20180919145318-efcd4e0f9787


what's the story with this replacement?

Without it we get:

$ go mod tidy go: downloading vbom.ml/util v0.0.0-20180919145318-efcd4e0f9787 github.com/openshift/cluster-kube-controller-manager-operator/cmd/cluster-kube-controller-manager-operator imports github.com/openshift/library-go/pkg/operator/staticpod/prune tested by github.com/openshift/library-go/pkg/operator/staticpod/prune.test imports vbom.ml/util/sortorder: unrecognized import path "vbom.ml/util": https fetch: Get "https://vbom.ml/util?go-get=1": dial tcp: lookup vbom.ml: no such host

This library is an dependency of go-library.

It looks like this library was moved to github some time ago, and I saw this replacement in few Openshift components.

I guess the vendoring hid the issue up until now?

yeah, i guess so, thanks for the explanation!

Should not it be pulled from the library-go go.mod file automatically? The replace line has been in the go.mod file for at least a year. What has changed we need to duplicate it in the KCM code from now on?

Running go mod tidy locally just changing api/library-go to release-4.14 branch works without the need to set the replace.

same it works for me with:

podman run --rm -v "$PWD":/usr/src/myapp -w /usr/src/myapp golang:1.20.4 sh -c "go mod tidy && go mod vendor && go mod verify"

but the one I have on my system (fedora) keeps failing 🤔

edit: go clean -modcache did not help

I can reproduce the failure with the fedora image:

podman run --rm -v "$PWD":/usr/src/myapp -w /usr/src/myapp fedora sh -c "dnf install -y golang && go mod tidy && go mod vendor && go mod verify"

GOPROXY is off by default on fedora, the following works:

GOPROXY=https://proxy.golang.org go mod tidy

Do you think we should keep the replace or not? because others will face the same issue I guess

Let's drop the replace for now. In case it will start causing trouble it can be always re-introduced. If the CI does not mind the replace is not needed.

elmiko · 2023-06-15T11:22:44Z

/lgtm

…port Revendor openshift/api and openshift/library-go in order to support the external platform type. library-go PR: openshift/library-go#1496 API PR: openshift/api#1434

ingvagabund · 2023-06-16T09:06:01Z

/lgtm
/approve

openshift-ci · 2023-06-16T09:06:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adriengentil, elmiko, ingvagabund

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~~ [ingvagabund]

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 · 2023-06-19T08:35:47Z

/retest-required

Remaining retests: 0 against base HEAD 1e15498 and 2 for PR HEAD 77fb2dd in total

openshift-ci · 2023-06-19T10:27:16Z

@adriengentil: all tests passed!

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-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 15, 2023

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 15, 2023

adriengentil mentioned this pull request May 15, 2023

OCPCLOUD-2010: Add CloudControllerManagerStatus to External platform status openshift/api#1434

Merged

adriengentil force-pushed the revendor-library-go branch from 9d41cee to 9256617 Compare May 22, 2023 13:56

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 1, 2023

adriengentil force-pushed the revendor-library-go branch from 9256617 to 6e16de5 Compare June 13, 2023 13:09

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2023

elmiko reviewed Jun 13, 2023

View reviewed changes

adriengentil force-pushed the revendor-library-go branch from 6e16de5 to 0016468 Compare June 15, 2023 08:17

adriengentil marked this pull request as ready for review June 15, 2023 08:17

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 15, 2023

openshift-ci bot requested review from ingvagabund and soltysh June 15, 2023 08:18

elmiko reviewed Jun 15, 2023

View reviewed changes

openshift-ci bot assigned elmiko Jun 15, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 15, 2023

OCPCLOUD-2010: Re-vendor api and library-go for external platform sup…

77fb2dd

…port Revendor openshift/api and openshift/library-go in order to support the external platform type. library-go PR: openshift/library-go#1496 API PR: openshift/api#1434

adriengentil force-pushed the revendor-library-go branch from 0016468 to 77fb2dd Compare June 16, 2023 08:10

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 16, 2023

openshift-ci bot assigned ingvagabund Jun 16, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 16, 2023

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 16, 2023

openshift-merge-robot merged commit 1807a0d into openshift:master Jun 19, 2023

Comments

Conversation

adriengentil commented May 15, 2023

Uh oh!

openshift-ci-robot commented May 15, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented May 15, 2023

Uh oh!

adriengentil commented May 16, 2023

Uh oh!

openshift-ci bot commented May 16, 2023

Uh oh!

adriengentil commented May 16, 2023

Uh oh!

elmiko left a comment

Choose a reason for hiding this comment

Uh oh!

elmiko left a comment

Choose a reason for hiding this comment

Uh oh!

elmiko Jun 15, 2023

Choose a reason for hiding this comment

Uh oh!

adriengentil Jun 15, 2023

Choose a reason for hiding this comment

Uh oh!

elmiko Jun 15, 2023

Choose a reason for hiding this comment

Uh oh!

ingvagabund Jun 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ingvagabund Jun 15, 2023

Choose a reason for hiding this comment

Uh oh!

adriengentil Jun 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adriengentil Jun 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adriengentil Jun 15, 2023

Choose a reason for hiding this comment

Uh oh!

ingvagabund Jun 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adriengentil Jun 16, 2023

Choose a reason for hiding this comment

Uh oh!

elmiko commented Jun 15, 2023

Uh oh!

ingvagabund commented Jun 16, 2023

Uh oh!

openshift-ci bot commented Jun 16, 2023

Uh oh!

openshift-ci-robot commented Jun 19, 2023

Uh oh!

openshift-ci bot commented Jun 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

openshift-ci-robot commented May 15, 2023 •

edited by openshift-ci bot

Loading

ingvagabund Jun 15, 2023 •

edited

Loading

adriengentil Jun 15, 2023 •

edited

Loading

adriengentil Jun 15, 2023 •

edited

Loading

ingvagabund Jun 15, 2023 •

edited

Loading