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

Switch EventType reconciler to v1beta1 version #3200

Merged
merged 8 commits into from
May 26, 2020
Merged

Switch EventType reconciler to v1beta1 version #3200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bb21ec9
generate v1beta reconciler
whaught May 23, 2020
e9be377
Merge remote-tracking branch 'upstream/master' into switch-version
whaught May 23, 2020
d78c48e
try to create propagate broker status method
whaught May 23, 2020
4e6b9d9
Create a conversion for brokerstatus
whaught May 23, 2020
f26a164
Fix test types
whaught May 23, 2020
be1171b
add PropagateV1Alpha1BrokerStatus unit test
whaught May 23, 2020
cd07f30
update code-gen
whaught May 23, 2020
e7b90b1
remove old generated reconciler
whaught May 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions pkg/apis/eventing/v1alpha1/broker_conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"

corev1 "k8s.io/api/core/v1"
"knative.dev/eventing/pkg/apis/eventing/v1beta1"
"knative.dev/pkg/apis"
)
Expand Down Expand Up @@ -60,3 +61,22 @@ func (sink *Broker) ConvertFrom(ctx context.Context, obj apis.Convertible) error
return fmt.Errorf("Unknown conversion, got: %T", source)
}
}

// PropagateV1Alpha1BrokerStatus propagates a v1alpha1 BrokerStatus to a v1beta1 EventTypeStatus
func PropagateV1Alpha1BrokerStatus(et *v1beta1.EventTypeStatus, bs *BrokerStatus) {
bc := brokerCondSet.Manage(bs).GetTopLevelCondition()
if bc == nil {
et.MarkBrokerNotConfigured()
return
}
switch {
case bc.Status == corev1.ConditionUnknown:
et.MarkBrokerUnknown(bc.Reason, bc.Message)
case bc.Status == corev1.ConditionTrue:
eventTypeCondSet.Manage(et).MarkTrue(EventTypeConditionBrokerReady)
case bc.Status == corev1.ConditionFalse:
et.MarkBrokerFailed(bc.Reason, bc.Message)
default:
et.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is invalid: %v", bc.Status)
}
}
54 changes: 54 additions & 0 deletions pkg/apis/eventing/v1alpha1/broker_conversion_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,60 @@ func TestBrokerConversion(t *testing.T) {
}
}

func TestBrokerPropagateStatus(t *testing.T) {
tests := []struct {
name string
markBrokerExists *bool
brokerStatus *BrokerStatus
wantConditionStatus corev1.ConditionStatus
}{{
name: "all happy",
markBrokerExists: &trueValue,
brokerStatus: TestHelper.ReadyBrokerStatus(),
wantConditionStatus: corev1.ConditionTrue,
}, {
name: "broker exist sad",
markBrokerExists: &falseValue,
brokerStatus: nil,
wantConditionStatus: corev1.ConditionFalse,
}, {
name: "broker ready sad",
markBrokerExists: &trueValue,
brokerStatus: TestHelper.FalseBrokerStatus(),
wantConditionStatus: corev1.ConditionFalse,
}, {
name: "broker ready unknown",
markBrokerExists: &trueValue,
brokerStatus: TestHelper.UnknownBrokerStatus(),
wantConditionStatus: corev1.ConditionUnknown,
}, {
name: "all sad",
markBrokerExists: &falseValue,
brokerStatus: nil,
wantConditionStatus: corev1.ConditionFalse,
}}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
ets := &v1beta1.EventTypeStatus{}
if test.markBrokerExists != nil {
if *test.markBrokerExists {
ets.MarkBrokerExists()
} else {
ets.MarkBrokerDoesNotExist()
}
}
if test.brokerStatus != nil {
PropagateV1Alpha1BrokerStatus(ets, test.brokerStatus)
}

got := ets.GetTopLevelCondition().Status
if test.wantConditionStatus != got {
t.Errorf("unexpected readiness: want %v, got %v", test.wantConditionStatus, got)
}
})
}
}

// Since v1beta1 to v1alpha1 is lossy but semantically equivalent,
// fix that so diff works.
func fixBrokerDeprecated(in *Broker) *Broker {
Expand Down
1 change: 0 additions & 1 deletion pkg/apis/eventing/v1alpha1/eventtype_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
)

// +genclient
// +genreconciler
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

type EventType struct {
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/eventing/v1beta1/eventtype_lifecycle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1beta1

import (
corev1 "k8s.io/api/core/v1"

"knative.dev/pkg/apis"
)

Expand Down
1 change: 1 addition & 0 deletions pkg/apis/eventing/v1beta1/eventtype_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
)

// +genclient
// +genreconciler
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

type EventType struct {
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/eventing/v1beta1/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ func addKnownTypes(scheme *runtime.Scheme) error {
scheme.AddKnownTypes(SchemeGroupVersion,
&Broker{},
&BrokerList{},
// &EventType{},
// &EventTypeList{},
&EventType{},
&EventTypeList{},
&Trigger{},
&TriggerList{},
)
Expand Down
2 changes: 1 addition & 1 deletion ...eventing/v1alpha1/eventtype/controller.go → .../eventing/v1beta1/eventtype/controller.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 18 additions & 18 deletions ...eventing/v1alpha1/eventtype/reconciler.go → .../eventing/v1beta1/eventtype/reconciler.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions ...ing/v1alpha1/eventtype/stub/controller.go → ...ting/v1beta1/eventtype/stub/controller.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions ...ing/v1alpha1/eventtype/stub/reconciler.go → ...ting/v1beta1/eventtype/stub/reconciler.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions pkg/reconciler/eventtype/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ import (

"knative.dev/pkg/logging"

"knative.dev/eventing/pkg/apis/eventing/v1alpha1"
"knative.dev/pkg/configmap"
"knative.dev/pkg/controller"
"knative.dev/pkg/tracker"

"knative.dev/eventing/pkg/apis/eventing/v1alpha1"
Copy link
Member

Choose a reason for hiding this comment

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

for here,

eventing/pkg/reconciler/eventtype/controller.go

Line 59 in cd07f30

v1alpha1.SchemeGroupVersion.WithKind("Broker"),

can we use beta1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

at the moment this is still using v1alpha1.Broker everywhere - this PR is intended only to update the EventType resource. Someone more familiar would have to explain if we ought to be reconciling v1beta1.Broker?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we're still reconciling with v1alpha1 for Brokers since v1beta1 is lossy. We'll drop v1alpha1 support and let's do the move to v1beta1 as part of that.

brokerinformer "knative.dev/eventing/pkg/client/injection/informers/eventing/v1alpha1/broker"
eventtypeinformer "knative.dev/eventing/pkg/client/injection/informers/eventing/v1alpha1/eventtype"
eventtypereconciler "knative.dev/eventing/pkg/client/injection/reconciler/eventing/v1alpha1/eventtype"
eventtypeinformer "knative.dev/eventing/pkg/client/injection/informers/eventing/v1beta1/eventtype"
eventtypereconciler "knative.dev/eventing/pkg/client/injection/reconciler/eventing/v1beta1/eventtype"
)

// NewController initializes the controller and is called by the generated code
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/eventtype/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (

// Fake injection informers
_ "knative.dev/eventing/pkg/client/injection/informers/eventing/v1alpha1/broker/fake"
_ "knative.dev/eventing/pkg/client/injection/informers/eventing/v1alpha1/eventtype/fake"
_ "knative.dev/eventing/pkg/client/injection/informers/eventing/v1beta1/eventtype/fake"
)

func TestNew(t *testing.T) {
Expand Down
16 changes: 9 additions & 7 deletions pkg/reconciler/eventtype/eventtype.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ import (
corev1 "k8s.io/api/core/v1"
apierrs "k8s.io/apimachinery/pkg/api/errors"
"knative.dev/eventing/pkg/apis/eventing/v1alpha1"
eventtypereconciler "knative.dev/eventing/pkg/client/injection/reconciler/eventing/v1alpha1/eventtype"
listers "knative.dev/eventing/pkg/client/listers/eventing/v1alpha1"
"knative.dev/eventing/pkg/apis/eventing/v1beta1"
eventtypereconciler "knative.dev/eventing/pkg/client/injection/reconciler/eventing/v1beta1/eventtype"
alphalisters "knative.dev/eventing/pkg/client/listers/eventing/v1alpha1"
betalisters "knative.dev/eventing/pkg/client/listers/eventing/v1beta1"
"knative.dev/eventing/pkg/logging"
pkgreconciler "knative.dev/pkg/reconciler"
"knative.dev/pkg/tracker"
Expand All @@ -38,8 +40,8 @@ func newReconciledNormal(namespace, name string) pkgreconciler.Event {

type Reconciler struct {
// listers index properties about resources
eventTypeLister listers.EventTypeLister
brokerLister listers.BrokerLister
eventTypeLister betalisters.EventTypeLister
brokerLister alphalisters.BrokerLister
tracker tracker.Interface
}

Expand All @@ -52,7 +54,7 @@ var _ eventtypereconciler.Interface = (*Reconciler)(nil)
// 1. Verify the Broker exists.
// 2. Verify the Broker is ready.
// TODO remove https://github.com/knative/eventing/issues/2750
func (r *Reconciler) ReconcileKind(ctx context.Context, et *v1alpha1.EventType) pkgreconciler.Event {
func (r *Reconciler) ReconcileKind(ctx context.Context, et *v1beta1.EventType) pkgreconciler.Event {
et.Status.InitializeConditions()
et.Status.ObservedGeneration = et.Generation

Expand Down Expand Up @@ -82,12 +84,12 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, et *v1alpha1.EventType)
return err
}

et.Status.PropagateBrokerStatus(&b.Status)
v1alpha1.PropagateV1Alpha1BrokerStatus(&et.Status, &b.Status)

return newReconciledNormal(et.Namespace, et.Name)
}

// getBroker returns the Broker for EventType 'et' if it exists, otherwise it returns an error.
func (r *Reconciler) getBroker(ctx context.Context, et *v1alpha1.EventType) (*v1alpha1.Broker, error) {
func (r *Reconciler) getBroker(ctx context.Context, et *v1beta1.EventType) (*v1alpha1.Broker, error) {
return r.brokerLister.Brokers(et.Namespace).Get(et.Spec.Broker)
}
Loading