search

knative / serving-operator

DEPRECATED: Development continues in https://github.com/knative/operator/
Apache License 2.0
39 stars 48 forks source link

Use Genreconciler for KnativeServing #380

Closed trshafer closed 4 years ago

trshafer commented 4 years ago

Fixes #335

Proposed Changes

Release Note

NONE
houshengbo commented 4 years ago

@trshafer @Cynocracy I have submitted this PR to move over all the serving-operator code: https://github.com/knative-sandbox/operator/pull/1 I am thinking of whether we FREEZE the operators' repo, to make sure all the code land in there. After two operators embrace each other in one repo, with correct functionalities, we can move forward with the new repo. Thoughts?

trshafer commented 4 years ago

@houshengbo genreconciler should make it easier to transition from one operator to another as more of the boilerplate code is autogenerated. I think we should finish #335. Thoughts?

Cynocracy commented 4 years ago

My opinion is that we should land this and a similar change in eventing to minimize the boilerplate / wiring needed to combine the operators. We have this working for serving, maybe we could prepare a similar change for eventing before we merge this?

trshafer commented 4 years ago

@houshengbo thoughts on merging this change? The next change will be to remove rbase: https://github.com/knative/serving-operator/blob/d0e771a0a4adb9893b9e7af9c9eb866f9dd937df/pkg/reconciler/knativeserving/controller.go#L57

houshengbo commented 4 years ago

/close We stopped contributing to this repo, as our new repo for knative operator has set up: knative-sandbox/operator.

knative-prow-robot commented 4 years ago

@houshengbo: Closed this PR.

In response to [this](https://github.com/knative/serving-operator/pull/380#issuecomment-611203769): >/close >We stopped contributing to this repo, as our new repo for knative operator has set up: knative-sandbox/operator. Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
houshengbo commented 4 years ago

/retest

trshafer commented 4 years ago

/hold

Will update with removing rbase.

trshafer commented 4 years ago

/cancel hold

trshafer commented 4 years ago

/hold cancel

lgtm-com[bot] commented 4 years ago

This pull request introduces 1 alert and fixes 1 when merging b8f656bf156afb045a41abec6e52fb3ee22e67ef into 940a4ca47efaa0a65e03d706412ed04d7c3e0ea0 - view on LGTM.com

new alerts:

fixed alerts:

knative-metrics-robot commented 4 years ago

The following is the coverage report on the affected files. Say /test pull-knative-serving-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/serving/v1alpha1/knativeserving_lifecycle.go 57.9% 52.9% -5.0
lgtm-com[bot] commented 4 years ago

This pull request fixes 1 alert when merging 28b217e2c9ac7b0b18fbb1fc1abda8a911a49e55 into 940a4ca47efaa0a65e03d706412ed04d7c3e0ea0 - view on LGTM.com

fixed alerts:

houshengbo commented 4 years ago

This PR is equivalent to the corresponding changes we had in knative-sandbox/operator, in terms of refactoring the reconciler. /lgtm /approve

knative-prow-robot commented 4 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: houshengbo, trshafer

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: - ~~[OWNERS](https://github.com/knative/serving-operator/blob/master/OWNERS)~~ [houshengbo,trshafer] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment