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

OCPBUGS-34399: Exposing chunksize variable to utilize docker registry config #1026

Merged
merged 2 commits into from
May 30, 2024
Merged

OCPBUGS-34399: Exposing chunksize variable to utilize docker registry config #1026

merged 2 commits into from
May 30, 2024

Conversation

deepsm007
Copy link
Contributor

@deepsm007 deepsm007 commented Apr 22, 2024 •
edited

Trying to work around a bug for r2 integration with s3 bucket, exposing chunksize variable by utilizing the docker registry S3 driver configuration option: https://docker-docs.uclv.cu/registry/storage-drivers/s3/

/hold for openshift/api#1864 to merge

https://issues.redhat.com//browse/IR-471

/cc @flavianmissi

@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 Apr 22, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Apr 22, 2024

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

@deepsm007
Copy link
Contributor Author

/test all

@deepsm007 deepsm007 changed the title Exposing chunksize variable to utilize WIP Exposing chunksize variable to utilize Apr 22, 2024
@deepsm007 deepsm007 changed the title WIP Exposing chunksize variable to utilize WIP Exposing chunksize variable to utilize docker registry config Apr 22, 2024
@deepsm007 deepsm007 mentioned this pull request Apr 22, 2024
Merged
@deepsm007
Copy link
Contributor Author

/test all

@flavianmissi
Copy link
Member

so far so good, I'll put it on hold until the o/api changes merge (your hold didn't take).
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 22, 2024
@deepsm007 deepsm007 force-pushed the expose-chunksize branch 2 times, most recently from 8471e70 to aaa90f5 Compare April 22, 2024 15:58
@deepsm007 deepsm007 changed the title WIP Exposing chunksize variable to utilize docker registry config Exposing chunksize variable to utilize docker registry config Apr 22, 2024
@deepsm007 deepsm007 marked this pull request as ready for review April 22, 2024 15:59
@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 Apr 22, 2024
@deepsm007 deepsm007 force-pushed the expose-chunksize branch 5 times, most recently from 547c9f5 to d6af07a Compare April 23, 2024 11:05
@deepsm007 deepsm007 force-pushed the expose-chunksize branch 8 times, most recently from e913bad to b34aad0 Compare May 1, 2024 21:15
flavianmissi
flavianmissi reviewed May 2, 2024
View reviewed changes
Copy link
Member

@flavianmissi flavianmissi left a comment

Choose a reason for hiding this comment

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

Could you separate feature gate support from the chunksize config commit? More specifically I'd like to see all the dependency injection updates as well as feature gate accessor initialization in the same commit. Thanks!

pkg/operator/starter.go Show resolved Hide resolved
@deepsm007
Copy link
Contributor Author

/test e2e-vsphere
/test e2e-vsphere-operator

@deepsm007
Copy link
Contributor Author

vsphere failures indicate that the feature gate detection code is interfering with graceful shutdown:

E0513 19:59:57.528191       1 starter.go:89] timed out waiting for FeatureGate detection
W0513 19:59:57.528346       1 builder.go:130] graceful termination failed, controllers failed with error: timed out waiting for FeatureGate detection

link to logs: https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_cluster-image-registry-operator/1026/pull-ci-openshift-cluster-image-registry-operator-master-e2e-vsphere/1790093611942023168/artifacts/e2e-vsphere/gather-extra/artifacts/pods/openshift-image-registry_cluster-image-registry-operator-65f88ff885-mk88n_cluster-image-registry-operator.log can you look into it @deepsm007 ? the operator needs to be able to shutdown gracefully as it previously did.

That run is from old commit, out of the last 4 runs 2 passed and 2 failed (not related to the PR).
This is the most recent run.

@deepsm007
Copy link
Contributor Author

/test e2e-vsphere

@flavianmissi
Copy link
Member

/retest

@deepsm007
Copy link
Contributor Author

/retest

1 similar comment
@deepsm007
Copy link
Contributor Author

/retest

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented May 30, 2024 •
edited

@deepsm007: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-openstack bb32361 link false /test e2e-openstack
ci/prow/e2e-vsphere a62f325 link false /test e2e-vsphere

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-sigs/prow repository. I understand the commands that are listed here.

@xiuwang
Copy link

xiuwang commented May 30, 2024

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label May 30, 2024
@flavianmissi
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 30, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented May 30, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deepsm007, flavianmissi

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 30, 2024
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 3c5fa91 and 2 for PR HEAD 1d09e26 in total

@flavianmissi
Copy link
Member

hypershift failures are unrelated
/override ci/prow/e2e-hypershift
/override ci/prow/e2e-hypershift-conformance

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented May 30, 2024

@flavianmissi: Overrode contexts on behalf of flavianmissi: ci/prow/e2e-hypershift, ci/prow/e2e-hypershift-conformance

In response to this:

hypershift failures are unrelated
/override ci/prow/e2e-hypershift
/override ci/prow/e2e-hypershift-conformance

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-sigs/prow repository.

@openshift-merge-bot openshift-merge-bot bot merged commit e5f344e into openshift:master May 30, 2024
12 checks passed
@openshift-ci-robot
Copy link
Contributor

@deepsm007: Jira Issue OCPBUGS-34399: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-34399 has been moved to the MODIFIED state.

In response to this:

Trying to work around a bug for r2 integration with s3 bucket, exposing chunksize variable by utilizing the docker registry S3 driver configuration option: https://docker-docs.uclv.cu/registry/storage-drivers/s3/

/hold for openshift/api#1864 to merge

https://issues.redhat.com//browse/IR-471

/cc @flavianmissi

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 openshift-eng/jira-lifecycle-plugin repository.

@deepsm007
Copy link
Contributor Author

/cherrypick release-4.16

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request May 30, 2024
Open
@openshift-cherrypick-robot
Copy link

@deepsm007: new pull request created: #1050

In response to this:

/cherrypick release-4.16

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-sigs/prow repository.

@openshift-bot
Copy link
Contributor

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-cluster-image-registry-operator-container-v4.17.0-202405301713.p0.ge5f344e.assembly.stream.el9 for distgit ose-cluster-image-registry-operator.
All builds following this will include this PR.

csrwng added a commit to csrwng/cluster-image-registry-operator that referenced this pull request May 30, 2024
@csrwng
Revert "Merge pull request openshift#1026 from deepsm007/expose-chunk…
8a2270a 
…size"

This reverts commit e5f344e, reversing
changes made to 3c5fa91.
@flavianmissi flavianmissi mentioned this pull request May 31, 2024
Merged
openshift-merge-bot bot added a commit that referenced this pull request May 31, 2024
@openshift-merge-bot
Merge pull request #1052 from csrwng/revert_1026
9ab6c4d 
OCPBUGS-34399:  Revert "Merge pull request #1026 from deepsm007/expose-chunksize"
deepsm007 added a commit to deepsm007/cluster-image-registry-operator that referenced this pull request May 31, 2024
@deepsm007
Revert "OCPBUGS-34399: Revert "Merge pull request openshift#1026 from…
4b07bfb 
… deepsm007/expose-chunksize""
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flavianmissi flavianmissi flavianmissi left review comments

@xiuwang xiuwang Awaiting requested review from xiuwang

Assignees

@flavianmissi flavianmissi

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. 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. 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
7 participants
@deepsm007 @flavianmissi @openshift-ci-robot @xiuwang @openshift-cherrypick-robot @openshift-bot @openshift-merge-robot