Skip to content

Commit

Permalink
Prepare for splitting configmap package. (knative#10425)
Browse files Browse the repository at this point in the history
  • Loading branch information
mattmoor authored Dec 22, 2020
1 parent 8d1562f commit f936091
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 74 deletions.
3 changes: 2 additions & 1 deletion cmd/activator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (

network "knative.dev/networking/pkg"
"knative.dev/pkg/configmap"
configmapinformer "knative.dev/pkg/configmap/informer"
"knative.dev/pkg/controller"
"knative.dev/pkg/injection/sharedmain"
pkglogging "knative.dev/pkg/logging"
Expand Down Expand Up @@ -155,7 +156,7 @@ func main() {
})

// Set up our config store
configMapWatcher := configmap.NewInformedWatcher(kubeClient, system.Namespace())
configMapWatcher := configmapinformer.NewInformedWatcher(kubeClient, system.Namespace())
configStore := activatorconfig.NewStore(logger, tracerUpdater)
configStore.WatchConfigs(configMapWatcher)

Expand Down
2 changes: 1 addition & 1 deletion cmd/autoscaler/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import (
"knative.dev/pkg/injection/sharedmain"
"knative.dev/pkg/leaderelection"

"knative.dev/pkg/configmap"
configmap "knative.dev/pkg/configmap/informer"
"knative.dev/pkg/controller"
"knative.dev/pkg/logging"
"knative.dev/pkg/metrics"
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ require (
knative.dev/caching v0.0.0-20201217015204-c3efd692dc36
knative.dev/hack v0.0.0-20201214230143-4ed1ecb8db24
knative.dev/networking v0.0.0-20201218160702-9c60344e49e5
knative.dev/pkg v0.0.0-20201218185703-e41409af6cff
knative.dev/pkg v0.0.0-20201222215804-e2d6b4f84573
sigs.k8s.io/yaml v1.2.0
)
41 changes: 2 additions & 39 deletions go.sum

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package configmap
package informer

import (
"errors"
Expand All @@ -31,14 +31,15 @@ import (
"k8s.io/client-go/informers/internalinterfaces"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/cache"
"knative.dev/pkg/configmap"
)

// NewInformedWatcherFromFactory watches a Kubernetes namespace for ConfigMap changes.
func NewInformedWatcherFromFactory(sif informers.SharedInformerFactory, namespace string) *InformedWatcher {
return &InformedWatcher{
sif: sif,
informer: sif.Core().V1().ConfigMaps(),
ManualWatcher: ManualWatcher{
ManualWatcher: configmap.ManualWatcher{
Namespace: namespace,
},
defaults: make(map[string]*corev1.ConfigMap),
Expand Down Expand Up @@ -97,22 +98,22 @@ type InformedWatcher struct {
// Embedding this struct allows us to reuse the logic
// of registering and notifying observers. This simplifies the
// InformedWatcher to just setting up the Kubernetes informer.
ManualWatcher
configmap.ManualWatcher
}

// Asserts that InformedWatcher implements Watcher.
var _ Watcher = (*InformedWatcher)(nil)
var _ configmap.Watcher = (*InformedWatcher)(nil)

// Asserts that InformedWatcher implements DefaultingWatcher.
var _ DefaultingWatcher = (*InformedWatcher)(nil)
var _ configmap.DefaultingWatcher = (*InformedWatcher)(nil)

// WatchWithDefault implements DefaultingWatcher.
func (i *InformedWatcher) WatchWithDefault(cm corev1.ConfigMap, o ...Observer) {
func (i *InformedWatcher) WatchWithDefault(cm corev1.ConfigMap, o ...configmap.Observer) {
i.defaults[cm.Name] = &cm

i.m.Lock()
i.Lock()
started := i.started
i.m.Unlock()
i.Unlock()
if started {
// TODO make both Watch and WatchWithDefault work after the InformedWatcher has started.
// This likely entails changing this to `o(&cm)` and having Watch check started, if it has
Expand All @@ -130,11 +131,12 @@ func (i *InformedWatcher) Start(stopCh <-chan struct{}) error {
// Pretend that all the defaulted ConfigMaps were just created. This is done before we start
// the informer to ensure that if a defaulted ConfigMap does exist, then the real value is
// processed after the default one.
for k := range i.observers {
i.ForEach(func(k string, _ []configmap.Observer) error {
if def, ok := i.defaults[k]; ok {
i.addConfigMapEvent(def)
}
}
return nil
})

if err := i.registerCallbackAndStartInformer(stopCh); err != nil {
return err
Expand All @@ -149,8 +151,8 @@ func (i *InformedWatcher) Start(stopCh <-chan struct{}) error {
}

func (i *InformedWatcher) registerCallbackAndStartInformer(stopCh <-chan struct{}) error {
i.m.Lock()
defer i.m.Unlock()
i.Lock()
defer i.Unlock()
if i.started {
return errors.New("watcher already started")
}
Expand All @@ -168,19 +170,19 @@ func (i *InformedWatcher) registerCallbackAndStartInformer(stopCh <-chan struct{
}

func (i *InformedWatcher) checkObservedResourcesExist() error {
i.m.RLock()
defer i.m.RUnlock()
i.RLock()
defer i.RUnlock()
// Check that all objects with Observers exist in our informers.
for k := range i.observers {
return i.ForEach(func(k string, _ []configmap.Observer) error {
if _, err := i.informer.Lister().ConfigMaps(i.Namespace).Get(k); err != nil {
if _, ok := i.defaults[k]; ok && k8serrors.IsNotFound(err) {
// It is defaulted, so it is OK that it doesn't exist.
continue
return nil
}
return err
}
}
return nil
return nil
})
}

func (i *InformedWatcher) addConfigMapEvent(obj interface{}) {
Expand Down
20 changes: 15 additions & 5 deletions vendor/knative.dev/pkg/configmap/manual_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,33 @@ type ManualWatcher struct {
Namespace string

// Guards observers
m sync.RWMutex
sync.RWMutex
observers map[string][]Observer
}

var _ Watcher = (*ManualWatcher)(nil)

// Watch implements Watcher
func (w *ManualWatcher) Watch(name string, o ...Observer) {
w.m.Lock()
defer w.m.Unlock()
w.Lock()
defer w.Unlock()

if w.observers == nil {
w.observers = make(map[string][]Observer, 1)
}
w.observers[name] = append(w.observers[name], o...)
}

// Watch implements Watcher
func (w *ManualWatcher) ForEach(f func(string, []Observer) error) error {
for k, v := range w.observers {
if err := f(k, v); err != nil {
return err
}
}
return nil
}

// Start implements Watcher
func (w *ManualWatcher) Start(<-chan struct{}) error {
return nil
Expand All @@ -55,8 +65,8 @@ func (w *ManualWatcher) OnChange(configMap *corev1.ConfigMap) {
return
}
// Within our namespace, take the lock and see if there are any registered observers.
w.m.RLock()
defer w.m.RUnlock()
w.RLock()
defer w.RUnlock()
// Iterate over the observers and invoke their callbacks.
for _, o := range w.observers[configMap.Name] {
o(configMap)
Expand Down
14 changes: 7 additions & 7 deletions vendor/knative.dev/pkg/injection/sharedmain/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import (
"k8s.io/client-go/rest"

kubeclient "knative.dev/pkg/client/injection/kube/client"
"knative.dev/pkg/configmap"
cminformer "knative.dev/pkg/configmap/informer"
"knative.dev/pkg/controller"
"knative.dev/pkg/injection"
"knative.dev/pkg/leaderelection"
Expand Down Expand Up @@ -290,26 +290,26 @@ func CheckK8sClientMinimumVersionOrDie(ctx context.Context, logger *zap.SugaredL

// SetupConfigMapWatchOrDie establishes a watch of the configmaps in the system
// namespace that are labeled to be watched or dies by calling log.Fatalw.
func SetupConfigMapWatchOrDie(ctx context.Context, logger *zap.SugaredLogger) *configmap.InformedWatcher {
func SetupConfigMapWatchOrDie(ctx context.Context, logger *zap.SugaredLogger) *cminformer.InformedWatcher {
kc := kubeclient.Get(ctx)
// Create ConfigMaps watcher with optional label-based filter.
var cmLabelReqs []labels.Requirement
if cmLabel := system.ResourceLabel(); cmLabel != "" {
req, err := configmap.FilterConfigByLabelExists(cmLabel)
req, err := cminformer.FilterConfigByLabelExists(cmLabel)
if err != nil {
logger.Fatalw("Failed to generate requirement for label "+cmLabel, zap.Error(err))
}
logger.Info("Setting up ConfigMap watcher with label selector: ", req)
cmLabelReqs = append(cmLabelReqs, *req)
}
// TODO(mattmoor): This should itself take a context and be injection-based.
return configmap.NewInformedWatcher(kc, system.Namespace(), cmLabelReqs...)
return cminformer.NewInformedWatcher(kc, system.Namespace(), cmLabelReqs...)
}

// WatchLoggingConfigOrDie establishes a watch of the logging config or dies by
// calling log.Fatalw. Note, if the config does not exist, it will be defaulted
// and this method will not die.
func WatchLoggingConfigOrDie(ctx context.Context, cmw *configmap.InformedWatcher, logger *zap.SugaredLogger, atomicLevel zap.AtomicLevel, component string) {
func WatchLoggingConfigOrDie(ctx context.Context, cmw *cminformer.InformedWatcher, logger *zap.SugaredLogger, atomicLevel zap.AtomicLevel, component string) {
if _, err := kubeclient.Get(ctx).CoreV1().ConfigMaps(system.Namespace()).Get(ctx, logging.ConfigMapName(),
metav1.GetOptions{}); err == nil {
cmw.Watch(logging.ConfigMapName(), logging.UpdateLevelFromConfigMap(logger, atomicLevel, component))
Expand All @@ -321,7 +321,7 @@ func WatchLoggingConfigOrDie(ctx context.Context, cmw *configmap.InformedWatcher
// WatchObservabilityConfigOrDie establishes a watch of the observability config
// or dies by calling log.Fatalw. Note, if the config does not exist, it will be
// defaulted and this method will not die.
func WatchObservabilityConfigOrDie(ctx context.Context, cmw *configmap.InformedWatcher, profilingHandler *profiling.Handler, logger *zap.SugaredLogger, component string) {
func WatchObservabilityConfigOrDie(ctx context.Context, cmw *cminformer.InformedWatcher, profilingHandler *profiling.Handler, logger *zap.SugaredLogger, component string) {
if _, err := kubeclient.Get(ctx).CoreV1().ConfigMaps(system.Namespace()).Get(ctx, metrics.ConfigMapName(),
metav1.GetOptions{}); err == nil {
cmw.Watch(metrics.ConfigMapName(),
Expand Down Expand Up @@ -351,7 +351,7 @@ func SecretFetcher(ctx context.Context) metrics.SecretFetcher {
// ControllersAndWebhooksFromCtors returns a list of the controllers and a list
// of the webhooks created from the given constructors.
func ControllersAndWebhooksFromCtors(ctx context.Context,
cmw *configmap.InformedWatcher,
cmw *cminformer.InformedWatcher,
ctors ...injection.ControllerConstructor) ([]*controller.Impl, []interface{}) {

// Check whether the context has been infused with a leader elector builder.
Expand Down
2 changes: 1 addition & 1 deletion vendor/knative.dev/pkg/tracing/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func SetupStaticPublishing(logger *zap.SugaredLogger, serviceName string, cfg *c
// just ensures that if generated, they are collected appropriately. This is normally done by using
// tracing.HTTPSpanMiddleware as a middleware HTTP handler. The configuration will be dynamically
// updated when the ConfigMap is updated.
func SetupDynamicPublishing(logger *zap.SugaredLogger, configMapWatcher *configmap.InformedWatcher, serviceName, tracingConfigName string) error {
func SetupDynamicPublishing(logger *zap.SugaredLogger, configMapWatcher configmap.Watcher, serviceName, tracingConfigName string) error {
oct := NewOpenCensusTracer(WithExporter(serviceName, logger))

tracerUpdater := func(name string, value interface{}) {
Expand Down
3 changes: 2 additions & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,7 @@ knative.dev/networking/test/conformance/ingress
knative.dev/networking/test/defaultsystem
knative.dev/networking/test/test_images/grpc-ping/proto
knative.dev/networking/test/types
# knative.dev/pkg v0.0.0-20201218185703-e41409af6cff
# knative.dev/pkg v0.0.0-20201222215804-e2d6b4f84573
## explicit
knative.dev/pkg/apiextensions/storageversion
knative.dev/pkg/apiextensions/storageversion/cmd/migrate
Expand Down Expand Up @@ -1065,6 +1065,7 @@ knative.dev/pkg/codegen/cmd/injection-gen/args
knative.dev/pkg/codegen/cmd/injection-gen/generators
knative.dev/pkg/configmap
knative.dev/pkg/configmap/hash-gen
knative.dev/pkg/configmap/informer
knative.dev/pkg/configmap/testing
knative.dev/pkg/controller
knative.dev/pkg/depcheck
Expand Down

0 comments on commit f936091

Please sign in to comment.