Skip to content

[WIP] OCPBUGS 2782: Update service deletion to retry with timeout #404

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

Closed
wants to merge 9 commits into from

Conversation

perdasilva
Copy link
Contributor

Signed-off-by: perdasilva [email protected]

Description of the change:
Adds a retry with timeout when attempting to delete a service during installation/upgrade.
This is a small mitigation for the bug linked in the title. The real fix for this would be to make install plan retryable. But given the efforts towards v1, it's probably not worth it.

Motivation for the change:
In the operator upgrade process, any services will be deleted and re-created. Sometimes these services can have finalizers attached that may delay the deletion of the resource and cause the upgrade to fail.

@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 Oct 24, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 24, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: perdasilva

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. labels Oct 24, 2022
tylerslaton and others added 9 commits October 24, 2022 16:52
Get the current OpenShift release version from the RELEASE_VERSION
environment variable since the behavior of the original source --
the ClusterVersion desired release status field -- has changed.

Signed-off-by: Tyler Slaton <[email protected]>
Co-authored-by: Nick Hale <[email protected]>
Upstream-repository: perdasilva
Upstream-commit: b2086bd2c5eb9b60370869757c51fa7908d2cde0
Signed-off-by: Jordan Keister <[email protected]>
Upstream-repository: perdasilva
Upstream-commit: c312b41e3079b5cb1672120046b376f50cb4246f
$ kubectl -n local get deployments
No resources found in local namespace.

$ kubectl -n olm get deployments
NAME               READY   UP-TO-DATE   AVAILABLE   AGE
catalog-operator   1/1     1            1           6m11s
olm-operator       1/1     1            1           10m
packageserver      1/1     1            1           10m

Signed-off-by: Brett Tofel <[email protected]>

Signed-off-by: Brett Tofel <[email protected]>
Upstream-repository: perdasilva
Upstream-commit: 665c25b3c42126b896791d8af9589fab2948080d
Signed-off-by: Tyler Slaton <[email protected]>

Signed-off-by: Tyler Slaton <[email protected]>
Upstream-repository: perdasilva
Upstream-commit: bd97e32644f5a6c5a2b87668df5261a4da62a2a6
This reverts commit bd97e32644f5a6c5a2b87668df5261a4da62a2a6.

An older v0.17.0 release of operator-framework/api had the spec.RunAsRoot
field, which has [now been updated to be the spec.GrpcPodConfig.SecurityContextConfig
field](operator-framework/api#261). Reverting #2848 so that
the new v0.17.0 can be pulled in. See [this comment](operator-framework/operator-lifecycle-manager#2848 (comment)) for more info.

Signed-off-by: Anik Bhattacharjee <[email protected]>

Signed-off-by: Anik Bhattacharjee <[email protected]>
Upstream-repository: perdasilva
Upstream-commit: bfeb1e46819bc840190f4d7d0720712683387973
Problem: The filterSecretsBySAName function attempts to identify all
service account token secrets related to a serviceAccount. To do so,
the filterSecretsBySAName function uses a range-for loop to iterate
over entries in the secrets argument. If a valid service account token
secret is found, a pointer to the range-for loop's value variable is
added to a map of results. Unfortunately, if a valid entry is found in
the middle of the list of secrets, the value returned by the range-for
loop is updated, causes the entry in the map to change.

Solution: Add a pointer to the actual secret instead of the range-for
loop's value variable.

Signed-off-by: Alexander Greene <[email protected]>

Signed-off-by: Alexander Greene <[email protected]>
Upstream-repository: perdasilva
Upstream-commit: caab6c52ec532dc82c7178eebb0377bd80d1e82a
The GOARCH flag was hardcoded to "386", which causes the binaries to not
work properly on machines with arm64 arch. This PR dynamically sets the
GOARCH flag to arm64 if the machine arch is arm64, or to 386 for all other
arch.

Signed-off-by: anik120 <[email protected]>

Signed-off-by: anik120 <[email protected]>
Upstream-repository: perdasilva
Upstream-commit: a8edd5c05a1dbeba19b80110b2d4df50ffb96c4c
Signed-off-by: perdasilva <[email protected]>
Upstream-repository: perdasilva
Upstream-commit: a1d68a4a02d22806d94cb29718a456db72ea3816
Signed-off-by: perdasilva <[email protected]>
Upstream-repository: perdasilva
Upstream-commit: fd1011590bdd0fb1200188bb26bffb76020a0024
@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 Oct 24, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 24, 2022

@perdasilva: 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/unit-olm 9be31fc link true /test unit-olm
ci/prow/e2e-gcp-olm-flaky 9be31fc link false /test e2e-gcp-olm-flaky
ci/prow/verify 9be31fc link true /test verify
ci/prow/e2e-gcp-olm 9be31fc link true /test e2e-gcp-olm

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

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 23, 2023
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 23, 2023
@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

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.

@perdasilva
Copy link
Contributor Author

closing and this does not seem like the direction we want to take

@perdasilva perdasilva closed this Feb 6, 2023
@perdasilva perdasilva deleted the ocpbugs-2782 branch February 6, 2023 09:58
openshift-bot pushed a commit to openshift-bot/operator-framework-olm that referenced this pull request Mar 15, 2025
Signed-off-by: Per Goncalves da Silva <[email protected]>
Co-authored-by: Per Goncalves da Silva <[email protected]>
Upstream-repository: api
Upstream-commit: 93aa09fdbe42fd43b195e4871edb671756a51610
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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants