Skip to content

Commit

Permalink
enable broker, use cm lister, prune trigger status
Browse files Browse the repository at this point in the history
  • Loading branch information
vaikas committed Jul 15, 2020
1 parent 4f2f82d commit e90c689
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 4 deletions.
27 changes: 26 additions & 1 deletion pkg/reconciler/mtbroker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"context"
"errors"
"fmt"
"reflect"
"time"

"go.uber.org/zap"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -67,6 +69,7 @@ type Reconciler struct {
endpointsLister corev1listers.EndpointsLister
subscriptionLister messaginglisters.SubscriptionLister
triggerLister eventinglisters.TriggerLister
configmapLister corev1listers.ConfigMapLister

channelableTracker duck.ListableTracker

Expand Down Expand Up @@ -213,7 +216,8 @@ func (r *Reconciler) getChannelTemplate(ctx context.Context, b *eventingv1.Broke
zap.String("namespace", b.Namespace), zap.String("name", b.Name))
return nil, errors.New("Broker.Spec.Config name and namespace are required")
}
cm, err := r.kubeClientSet.CoreV1().ConfigMaps(b.Spec.Config.Namespace).Get(b.Spec.Config.Name, metav1.GetOptions{})

cm, err := r.configmapLister.ConfigMaps(b.Spec.Config.Namespace).Get(b.Spec.Config.Name)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -363,3 +367,24 @@ func (r *Reconciler) propagateBrokerStatusToTriggers(ctx context.Context, namesp
}
return nil
}

// lifted from pkg/reconciler/reconcile_common for now...
// groomConditionsTransitionTime ensures that the LastTransitionTime only advances for resources
// where the condition has changed during reconciliation. This also ensures that all advanced
// conditions share the same timestamp.
func groomConditionsTransitionTime(resource, oldResource pkgduckv1.KRShaped) {
now := apis.VolatileTime{Inner: metav1.NewTime(time.Now())}
sts := resource.GetStatus()
for i := range sts.Conditions {
cond := &sts.Conditions[i]

if oldCond := oldResource.GetStatus().GetCondition(cond.Type); oldCond != nil {
cond.LastTransitionTime = oldCond.LastTransitionTime
if reflect.DeepEqual(cond, oldCond) {
continue
}
}

cond.LastTransitionTime = now
}
}
5 changes: 3 additions & 2 deletions pkg/reconciler/mtbroker/broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,11 @@ func TestReconcile(t *testing.T) {
WithBrokerClass(eventing.MTChannelBrokerClassValue),
WithInitBrokerConditions,
WithBrokerConfig(config()),
WithTriggerChannelFailed("ChannelTemplateFailed", `Error on setting up the ChannelTemplate: configmaps "test-configmap" not found`)),
WithTriggerChannelFailed("ChannelTemplateFailed", `Error on setting up the ChannelTemplate: configmap "test-configmap" not found`)),
}},
WantEvents: []string{
finalizerUpdatedEvent,
Eventf(corev1.EventTypeWarning, "InternalError", `configmaps "test-configmap" not found`),
Eventf(corev1.EventTypeWarning, "InternalError", `configmap "test-configmap" not found`),
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(testNS, brokerName),
Expand Down Expand Up @@ -1167,6 +1167,7 @@ func TestReconcile(t *testing.T) {
triggerLister: listers.GetTriggerLister(),

endpointsLister: listers.GetEndpointsLister(),
configmapLister: listers.GetConfigMapLister(),
kresourceTracker: duck.NewListableTracker(ctx, conditions.Get, func(types.NamespacedName) {}, 0),
channelableTracker: duck.NewListableTracker(ctx, channelable.Get, func(types.NamespacedName) {}, 0),
addressableTracker: duck.NewListableTracker(ctx, v1a1addr.Get, func(types.NamespacedName) {}, 0),
Expand Down
3 changes: 3 additions & 0 deletions pkg/reconciler/mtbroker/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"knative.dev/pkg/client/injection/ducks/duck/v1/addressable"
"knative.dev/pkg/client/injection/ducks/duck/v1/conditions"
kubeclient "knative.dev/pkg/client/injection/kube/client"
configmapinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap"
endpointsinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/endpoints"
"knative.dev/pkg/configmap"
"knative.dev/pkg/controller"
Expand Down Expand Up @@ -68,6 +69,7 @@ func NewController(
triggerInformer := triggerinformer.Get(ctx)
subscriptionInformer := subscriptioninformer.Get(ctx)
endpointsInformer := endpointsinformer.Get(ctx)
configmapInformer := configmapinformer.Get(ctx)

eventingv1.RegisterAlternateBrokerConditionSet(apis.NewLivingConditionSet(
BrokerConditionIngress,
Expand All @@ -84,6 +86,7 @@ func NewController(
subscriptionLister: subscriptionInformer.Lister(),
triggerLister: triggerInformer.Lister(),
brokerClass: eventing.MTChannelBrokerClassValue,
configmapLister: configmapInformer.Lister(),
}
impl := brokerreconciler.NewImpl(ctx, r, eventing.MTChannelBrokerClassValue)

Expand Down
1 change: 1 addition & 0 deletions pkg/reconciler/mtbroker/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
_ "knative.dev/pkg/client/injection/ducks/duck/v1/addressable/fake"
_ "knative.dev/pkg/client/injection/ducks/duck/v1/conditions/fake"
_ "knative.dev/pkg/client/injection/kube/informers/apps/v1/deployment/fake"
_ "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake"
_ "knative.dev/pkg/client/injection/kube/informers/core/v1/endpoints/fake"
_ "knative.dev/pkg/client/injection/kube/informers/core/v1/service/fake"
)
Expand Down
3 changes: 3 additions & 0 deletions pkg/reconciler/mtbroker/trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ func (r *Reconciler) updateTriggerStatus(ctx context.Context, desired *eventingv
return nil, err
}

// Do the postprocessing for unnecessary trigger status changes.
groomConditionsTransitionTime(desired, trigger)

if reflect.DeepEqual(trigger.Status, desired.Status) {
return trigger, nil
}
Expand Down
2 changes: 1 addition & 1 deletion test/config/chaosduck.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ spec:

args: [
# TODO(https://github.com/knative/eventing/issues/3591): Enable once MT Broker chaos issues are fixed.
"-disable=mt-broker-controller",
#"-disable=mt-broker-controller",
# TODO(https://github.com/knative/eventing/issues/3590): Enable once IMC chaos issues are fixed.
"-disable=imc-controller", "-disable=imc-dispatcher",
]
Expand Down
46 changes: 46 additions & 0 deletions test/config/config-logging.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Copyright 2020 The Knative Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

apiVersion: v1
kind: ConfigMap
metadata:
name: config-logging
namespace: knative-eventing
labels:
eventing.knative.dev/release: devel

data:
zap-logger-config: |
{
"level": "debug",
"development": false,
"outputPaths": ["stdout"],
"errorOutputPaths": ["stderr"],
"encoding": "json",
"encoderConfig": {
"timeKey": "ts",
"levelKey": "level",
"nameKey": "logger",
"callerKey": "caller",
"messageKey": "msg",
"stacktraceKey": "stacktrace",
"lineEnding": "",
"levelEncoder": "",
"timeEncoder": "iso8601",
"durationEncoder": "",
"callerEncoder": ""
}
}
# Log level overrides
loglevel.my-broker-controller: "debug"
5 changes: 5 additions & 0 deletions test/e2e-common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,11 @@ function install_sugar() {
}

function unleash_duck() {
echo "enable debug logging"
cat test/config/config-logging.yaml | \
sed "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${TEST_EVENTING_NAMESPACE}/g" | \
ko apply --strict -f - || return $?

echo "unleash the duck"
cat test/config/chaosduck.yaml | \
sed "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${TEST_EVENTING_NAMESPACE}/g" | \
Expand Down

0 comments on commit e90c689

Please sign in to comment.