Skip to content

Comments

[OCPCLOUD-1024]: Refactor webhook syncronization to work with library-go implementation#642

Merged
openshift-merge-robot merged 2 commits intoopenshift:masterfrom
Danil-Grigorev:webhook-apply-library-go
Dec 7, 2020
Merged

[OCPCLOUD-1024]: Refactor webhook syncronization to work with library-go implementation#642
openshift-merge-robot merged 2 commits intoopenshift:masterfrom
Danil-Grigorev:webhook-apply-library-go

Conversation

@Danil-Grigorev
Copy link

@Danil-Grigorev Danil-Grigorev commented Jul 15, 2020 •
edited
Loading

This PR removes our webhook management code in favor of openshift/library-go#836

It also removes unit tests, which are currently impossible to mock as it depends on generation increments, which is not supported even by testenv standards. But we have all the necessary infrastructure in
https://github.com/openshift/cluster-api-actuator-pkg/blob/master/pkg/operators/machine-api-operator.go#L75-L193 which extensively covers the code and typical use cases, and it is covered in openshift/library-go#836 which this PR is pulling.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 15, 2020
@Danil-Grigorev Danil-Grigorev force-pushed the webhook-apply-library-go branch from 175336c to fb4b853 Compare July 15, 2020 14:52
@Danil-Grigorev Danil-Grigorev mentioned this pull request Jul 15, 2020
Merged
@Danil-Grigorev Danil-Grigorev force-pushed the webhook-apply-library-go branch from fb4b853 to 3077008 Compare July 16, 2020 10:44
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 16, 2020

@Danil-Grigorev: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws 3077008 link /test e2e-aws
ci/prow/e2e-azure-operator 3077008 link /test e2e-azure-operator

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 openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 18, 2020
@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 28, 2020

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-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 28, 2020
@Danil-Grigorev Danil-Grigorev force-pushed the webhook-apply-library-go branch from 3077008 to 1a6ee55 Compare December 1, 2020 16:48
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 1, 2020
@Danil-Grigorev
Copy link
Author

Danil-Grigorev commented Dec 1, 2020

/retest

@Danil-Grigorev Danil-Grigorev force-pushed the webhook-apply-library-go branch from 1a6ee55 to bc5b448 Compare December 7, 2020 16:26
@Danil-Grigorev Danil-Grigorev changed the title [WIP] Refactor webhook syncronization to work with library-go implementation Refactor webhook syncronization to work with library-go implementation Dec 7, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 7, 2020
@Danil-Grigorev
Copy link
Author

Danil-Grigorev commented Dec 7, 2020

/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 7, 2020
@michaelgugino
Copy link
Contributor

michaelgugino commented Dec 7, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2020
@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Dec 7, 2020 •
edited
Loading

@Danil-Grigorev: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-metal-ipi bc5b448 link /test e2e-metal-ipi
ci/prow/e2e-azure bc5b448 link /test e2e-azure
ci/prow/e2e-gcp bc5b448 link /test e2e-gcp

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.

elmiko
elmiko approved these changes Dec 7, 2020
View reviewed changes
Copy link
Contributor

@elmiko elmiko left a comment

Choose a reason for hiding this comment

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

/approve

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 7, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: elmiko

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:

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 openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 7, 2020
@openshift-merge-robot openshift-merge-robot merged commit 44f29cc into openshift:master Dec 7, 2020
@Danil-Grigorev Danil-Grigorev deleted the webhook-apply-library-go branch December 7, 2020 21:25
@Danil-Grigorev Danil-Grigorev changed the title Refactor webhook syncronization to work with library-go implementation [OCPCLOUD-1024]: Refactor webhook syncronization to work with library-go implementation Dec 8, 2020
@Danil-Grigorev Danil-Grigorev mentioned this pull request Dec 15, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexander-demicev alexander-demicev Awaiting requested review from alexander-demicev

@michaelgugino michaelgugino Awaiting requested review from michaelgugino

1 more reviewer

@elmiko elmiko elmiko approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@michaelgugino michaelgugino

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Danil-Grigorev @openshift-ci-robot @openshift-bot @michaelgugino @openshift-merge-robot @elmiko