Skip to content

OPRUN-2913: Sync api, operator-registry, operator-lifecycle-manager downstream #510

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

Merged
merged 105 commits into from
Jul 26, 2023

Conversation

tmshort
Copy link
Contributor

@tmshort tmshort commented Jul 17, 2023

This is a big merge of the upstream repositories into the downstream mono-repo.

Due to different levels of activity in each repository, they were at different level of the k8s API, and this had to be resolved upstream before downstreaming. This involved updated quite a number of components in go.mod/go.sum.

In order to do this sync as cleanly as possible, because various bug fixes were downstreamed independently, I updated the sync scripts in the scripts directory. The first change was to interleave (based on the commit timestamp) the syncing of individual commits from the source repositories, in the future hope that it will make large merges like this easier. Doing each source repo one-at-a-time can cause go.mod components to be upgraded by one repo, and then regressed by another repo, before being upgraded again. Interleaving the commits from the repositories based on date helps alleviate this scenario.

The second change was to note if there were any changes to the go.mod files, and if so, give the merger (e.g. me in this case) the opportunity (by pausing) to evaluate if there were any regressions in the go.mod file. Then the merger can open a shell in another window to evaluate the go.mod file to see if it needs to be modified and/or reverted.

The third change was to handle failures of make manifests in a similar way. If make manifests fails, the update scripts will pause to allow the merge to evaluate the results. This was especially useful as make manifests was failing due to the inconsistent versioning of the k8s API in each of the source repositories.

In this merge, the k8s APIs are now at v0.27.2.

@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 17, 2023

@tmshort: This pull request references OPRUN-2913 which is a valid jira issue.

In response to this:

This is a big merge of the upstream repositories into the downstream mono-repo.

Due to different levels of activity in each repository, they were at different level of the k8s API, and this had to be resolved upstream before downstreaming. This involved updated quite a number of components in go.mod/go.sum.

In order to do this sync as cleanly as possible, because various bug fixes were downstreamed independently, I updated the sync scripts in the scripts directory. The first change was to interleave (based on the commit timestamp) the syncing of individual commits from the source repositories, in the future hope that it will make large merges like this easier. Doing each source repo one-at-a-time can cause go.mod components to be upgraded by one repo, and then regressed by another repo, before being upgraded again. Interleaving the commits from the repositories based on date helps alleviate this scenario.

The second change was to note if there were any changes to the go.mod files, and if so, give the merger (e.g. me in this case) the opportunity (by pausing) to evaluate if there were any regressions in the go.mod file. Then the merger can open a shell in another window to evaluate the go.mod file to see if it needs to be modified and/or reverted.

The third change was to handle failures of make manifests in a similar way. If make manifests fails, the update scripts will pause to allow the merge to evaluate the results. This was especially useful as make manifests was failing due to the inconsistent versioning of the k8s API in each of the source repositories.

In this merge, the k8s APIs are now at v0.27.2.

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 openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 17, 2023
@openshift-ci 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 Jul 17, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 17, 2023

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

@tmshort
Copy link
Contributor Author

tmshort commented Jul 17, 2023

/test all

@openshift-ci openshift-ci bot added the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Jul 17, 2023
This was referenced Jul 17, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 17, 2023

@tmshort: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test e2e-gcp-console-olm
  • /test e2e-gcp-olm
  • /test e2e-gcp-ovn
  • /test e2e-upgrade
  • /test images
  • /test unit-api
  • /test unit-olm
  • /test unit-psm
  • /test unit-registry
  • /test verify

The following commands are available to trigger optional jobs:

  • /test e2e-gcp-olm-flaky

Use /test all to run all jobs.

In response to this:

/retest all

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.

@tmshort
Copy link
Contributor Author

tmshort commented Jul 17, 2023

/retest

@tmshort tmshort marked this pull request as ready for review July 17, 2023 21:05
@openshift-ci 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 Jul 17, 2023
@openshift-ci openshift-ci bot requested review from ankitathomas and benluddy July 17, 2023 21:06
@perdasilva
Copy link
Contributor

/retest

1 similar comment
@perdasilva
Copy link
Contributor

/retest

@tmshort
Copy link
Contributor Author

tmshort commented Jul 18, 2023

It appears ci/prow/e2e-gcp-ovn/ci/prow/e2e-gcp-olm are failing because oc get packagemanifest (singular) is failing to retrieve any resources.
Creating a cluster with this PR, oc get packagemanifests (plural) works, but oc get packagemanifest (singular) fails. I will need to test a cluster w/o this PR to see which one works.
One of the upstream commits required the addition of GetSingularName() for packagemanifests, which you think would not be a problem (and would certainly not cause this problem).

@tmshort
Copy link
Contributor Author

tmshort commented Jul 18, 2023

With this PR

tshort@cube:~/OPRUN-2913$ oc get packagemanifests -A | wc
    432    2279   44063
tshort@cube:~/OPRUN-2913$ oc get packagemanifest -A | wc
error: the server doesn't have a resource type "packagemanifest"
      0       0       0
tshort@cube:~/OPRUN-2913$

@tmshort
Copy link
Contributor Author

tmshort commented Jul 18, 2023

Vanilla 4.14:

tshort@cube:~/OPRUN-2913$ oc get packagemanifests | wc
    432    1847   33264
tshort@cube:~/OPRUN-2913$ oc get packagemanifest | wc
    432    1847   33264
tshort@cube:~/OPRUN-2913$

@tmshort
Copy link
Contributor Author

tmshort commented Jul 18, 2023

It's a change in the upstream behavior; verified on a vanilla 4.14 cluster, and on operator-framework/operator-lifecycle-manager@11b66ef (tip of master) vs operator-framework/operator-lifecycle-manager@4564b26 (before k8s updates)

@tmshort tmshort force-pushed the sync-2023-07-17 branch 2 times, most recently from 648b33b to 0ea0616 Compare July 19, 2023 13:51
@tmshort
Copy link
Contributor Author

tmshort commented Jul 19, 2023

/retest

1 similar comment
@tmshort
Copy link
Contributor Author

tmshort commented Jul 19, 2023

/retest

Copy link
Contributor

@ankitathomas ankitathomas left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 20, 2023
@tmshort
Copy link
Contributor Author

tmshort commented Jul 20, 2023

The CI complains about the OWNERS files are all about those in the staging directories. Do we care? Should I remove those individuals, or even the OWNERS files themselves?

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 20, 2023
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 20, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 20, 2023

The following users are mentioned in OWNERS file(s) but are untrusted for the following reasons. One way to make the user trusted is to add them as members of the openshift org. You can then trigger verification by writing /verify-owners in a comment.

  • akihikokuroda
    • User is not a member of the org. User is not a collaborator. Satisfy at least one of these conditions to make the user trusted.
  • asmacdo
    • User is not a member of the org. User is not a collaborator. Satisfy at least one of these conditions to make the user trusted.
  • exdx
    • User is not a member of the org. User is not a collaborator. Satisfy at least one of these conditions to make the user trusted.
  • njhale
    • User is not a member of the org. User is not a collaborator. Satisfy at least one of these conditions to make the user trusted.
    • staging/operator-registry/OWNERS

@tmshort
Copy link
Contributor Author

tmshort commented Jul 21, 2023

/retest

@87utierrez
Copy link

@tmshort Hi, my name is Hector, I was hoping that you could help and share with me as I'm new with go and development. with run a build to create an image from this repo.

I've cloned this repo and made some changes to the go. mod and run the following command

  • go mod tidy
  • go mod verify
  • go mod vendor
  • go test ./...

It came back with the program, not an importable package.

  • what I'm trying to accomplish is to use this registry - within my docker file
    FROM registry.redhat.io/openshift4/ose-operator-registry:v4.13.0-202306072029.p0.gce46f5b.assembly.stream
    I'm pulling the /bin/opm and /bin/grpc_health_probe into my image. The reason why I made changes to the go mod list is to update the packages that have fixes from the previous version.

The ask is after making those changes and updating the go.sum , how do I rebuild the manifest to have an image made?
I could really use your help --- desperate.

tmshort and others added 2 commits July 24, 2023 10:17
Update pakcage-server-manager to support latest APIs/golang

Signed-off-by: Todd Short <[email protected]>
Otherwise, `oc get packagemanifest` (singular) breaks.
`oc get packagemanifests` (plural) works.

Signed-off-by: Todd Short <[email protected]>
Upstream-repository: operator-lifecycle-manager
Upstream-commit: b30a3be15c6a1df17292f1b873c2dd85a1fb41c2
@openshift-ci openshift-ci bot removed the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Jul 24, 2023
@tmshort
Copy link
Contributor Author

tmshort commented Jul 24, 2023

@tmshort Hi, my name is Hector, I was hoping that you could help and share with me as I'm new with go and development. with run a build to create an image from this repo.

Hi @87utierrez, this is not an appropriate forum for this type of discussion. I suggest joining the #olm-dev channel in the Kubernetes Slack instance: https://kubernetes.slack.com/archives/C0181L6JYQ2 and asking your questions there.

Copy link
Contributor

@ankitathomas ankitathomas left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 24, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 24, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ankitathomas, tmshort

The full list of commands accepted by this bot can be found here.

The pull request process is described here

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

@tmshort
Copy link
Contributor Author

tmshort commented Jul 24, 2023

/retest

1 similar comment
@tmshort
Copy link
Contributor Author

tmshort commented Jul 24, 2023

/retest

@87utierrez
Copy link

87utierrez commented Jul 24, 2023 via email

@tmshort
Copy link
Contributor Author

tmshort commented Jul 24, 2023

/retest

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD b4ce064 and 2 for PR HEAD 045a139 in total

@tmshort
Copy link
Contributor Author

tmshort commented Jul 25, 2023

/retest

1 similar comment
@m1kola
Copy link
Contributor

m1kola commented Jul 25, 2023

/retest

@m1kola
Copy link
Contributor

m1kola commented Jul 25, 2023

@87utierrez you can create a slack account and joint the community using this link - https://slack.k8s.io/

@tmshort
Copy link
Contributor Author

tmshort commented Jul 25, 2023

/retest

6 similar comments
@tmshort
Copy link
Contributor Author

tmshort commented Jul 25, 2023

/retest

@tmshort
Copy link
Contributor Author

tmshort commented Jul 25, 2023

/retest

@tmshort
Copy link
Contributor Author

tmshort commented Jul 26, 2023

/retest

@tmshort
Copy link
Contributor Author

tmshort commented Jul 26, 2023

/retest

@tmshort
Copy link
Contributor Author

tmshort commented Jul 26, 2023

/retest

@tmshort
Copy link
Contributor Author

tmshort commented Jul 26, 2023

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 26, 2023

@tmshort: all tests passed!

Full PR test history. Your PR dashboard.

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 9a977fa into openshift:master Jul 26, 2023
@tmshort tmshort deleted the sync-2023-07-17 branch July 27, 2023 17:15
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. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.