Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Avinash Patnala <avinashpatnala@google.com>
  • Loading branch information
Avinash Patnala committed Sep 20, 2024
1 parent 2ca5b9f commit 60ec8d5
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 6 deletions.
2 changes: 1 addition & 1 deletion pkg/controller/config/config_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque
// sync anything
gvksToSync := []schema.GroupVersionKind{}

// deleted is true if the instance doesn't exist or has a deletion timestamp.
// K8s API conventions consider an object to be deleted when either the object no longer exists or when a deletion timestamp has been set.
deleted := !exists || !instance.GetDeletionTimestamp().IsZero()

if !deleted {
Expand Down
12 changes: 7 additions & 5 deletions pkg/controller/configstatus/configstatus_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
configv1alpha1 "github.com/open-policy-agent/gatekeeper/v3/apis/config/v1alpha1"
"github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1"
"github.com/open-policy-agent/gatekeeper/v3/pkg/logging"
"github.com/open-policy-agent/gatekeeper/v3/pkg/operations"
"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
"github.com/open-policy-agent/gatekeeper/v3/pkg/util"
"github.com/open-policy-agent/gatekeeper/v3/pkg/watch"
Expand Down Expand Up @@ -52,6 +53,9 @@ func (a *Adder) InjectTracker(_ *readiness.Tracker) {}
// Add creates a new config Status Controller and adds it to the Manager. The Manager will set fields on the Controller
// and Start it when the Manager is Started.
func (a *Adder) Add(mgr manager.Manager) error {
if !operations.IsAssigned(operations.Status) {
return nil
}
r := newReconciler(mgr)
return add(mgr, r)
}
Expand All @@ -74,7 +78,7 @@ type PackerMap func(obj client.Object) []reconcile.Request
// PodStatusToConfigMapper correlates a ConfigPodStatus with its corresponding Config.
// `selfOnly` tells the mapper to only map statuses corresponding to the current pod.
func PodStatusToConfigMapper(selfOnly bool, packerMap handler.MapFunc) handler.TypedMapFunc[*v1beta1.ConfigPodStatus] {
return func(ctx context.Context, obj *v1beta1.ConfigPodStatus) []reconcile.Request {
return func(_ context.Context, obj *v1beta1.ConfigPodStatus) []reconcile.Request {
labels := obj.GetLabels()
name, ok := labels[v1beta1.ConfigNameLabel]
if !ok {
Expand All @@ -95,22 +99,21 @@ func PodStatusToConfigMapper(selfOnly bool, packerMap handler.MapFunc) handler.T
}
}

// add adds a new Controller to mgr with r as the reconcile.Reconciler.
// Add creates a new config status Controller and adds it to the Manager. The Manager will set fields on the Controller
// and Start it when the Manager is Started.
func add(mgr manager.Manager, r reconcile.Reconciler) error {
// Create a new controller
c, err := controller.New("config-status-controller", mgr, controller.Options{Reconciler: r})
if err != nil {
return err
}

// Watch for changes to ConfigStatus
err = c.Watch(
source.Kind(mgr.GetCache(), &v1beta1.ConfigPodStatus{}, handler.TypedEnqueueRequestsFromMapFunc(PodStatusToConfigMapper(false, util.EventPackerMapFunc()))))
if err != nil {
return err
}

// Watch for changes to Config
err = c.Watch(source.Kind(mgr.GetCache(), &configv1alpha1.Config{}, &handler.TypedEnqueueRequestForObject[*configv1alpha1.Config]{}))
if err != nil {
return err
Expand Down Expand Up @@ -160,7 +163,6 @@ func (r *ReconcileConfigStatus) Reconcile(ctx context.Context, request reconcile
sort.Sort(statusObjs)

var s []v1beta1.ConfigPodStatusStatus
// created is true if at least one Pod hasn't reported any errors

for i := range statusObjs {
// Don't report status if it's not for the correct object. This can happen
Expand Down
11 changes: 11 additions & 0 deletions pkg/readiness/ready_tracker_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,17 @@ const (
tick = 1 * time.Second
)

type WrapFakeClientWithMutex struct {

Check failure on line 52 in pkg/readiness/ready_tracker_unit_test.go

View workflow job for this annotation

GitHub Actions / Unit test

WrapFakeClientWithMutex redeclared in this block
listMutex sync.Mutex
fakeLister Lister
}

func (w *WrapFakeClientWithMutex) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {

Check failure on line 57 in pkg/readiness/ready_tracker_unit_test.go

View workflow job for this annotation

GitHub Actions / Unit test

method WrapFakeClientWithMutex.List already declared at pkg/readiness/ready_tracker.go:97:35
w.listMutex.Lock()
defer w.listMutex.Unlock()
return w.fakeLister.List(ctx, list, opts...)
}

var (
testConstraintTemplate = templates.ConstraintTemplate{
ObjectMeta: v1.ObjectMeta{
Expand Down

0 comments on commit 60ec8d5

Please sign in to comment.