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.

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

chore: feature flags logger #566

Merged
merged 4 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ func main() {
setupLog.Info("starting manager")
ctx := ctrl.SetupSignalHandler()

if err := feature.Initialize(ctx, feature.WithLogger(setupLog.WithName("ff"))); err != nil {
if err := feature.Initialize(ctx, rootLogger.WithName("ff")); err != nil {
setupLog.Error(err, "problem initializing feature flags")
}

Expand Down
21 changes: 21 additions & 0 deletions internal/controller/cloud-control/nfsinstance_gcp_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package cloudcontrol

import (
"context"
"github.com/kyma-project/cloud-manager/pkg/composed"
"time"

cloudcontrolv1beta1 "github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1"
Expand Down Expand Up @@ -436,6 +438,25 @@ var _ = Describe("Feature: KCP NFSVolume for GCP", func() {
infra.GcpMock().SetOperationError(nil)
})

It("// cleanup: delete GCP NfsVolume for unhappy path", func() {
infra.GcpMock().SetCreateError(nil)
infra.GcpMock().SetPatchError(nil)
infra.GcpMock().SetDeleteError(nil)
infra.GcpMock().SetGetError(nil)
infra.GcpMock().SetOperationError(nil)
Eventually(Delete).
WithArguments(infra.Ctx(), infra.KCP().Client(), gcpNfsInstance).
Should(Succeed())
Eventually(func(ctx context.Context) error {
_, err := composed.PatchObjRemoveFinalizer(ctx, cloudcontrolv1beta1.FinalizerName, gcpNfsInstance, infra.KCP().Client())
return err
}).
WithContext(infra.Ctx()).
Should(Succeed())
Eventually(IsDeleted).
WithArguments(infra.Ctx(), infra.KCP().Client(), gcpNfsInstance).
Should(Succeed())
})
})

Context("Scenario: GCP NFSVolume With Restore Happy Path", Ordered, func() {
Expand Down
8 changes: 8 additions & 0 deletions pkg/common/ignorant/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ignorant

import (
ctrl "sigs.k8s.io/controller-runtime"
"sync"
)

type Ignorant interface {
Expand All @@ -19,18 +20,25 @@ func New() Ignorant {
var _ Ignorant = &ignorant{}

type ignorant struct {
m sync.Mutex
nameMap map[string]struct{}
}

func (i *ignorant) AddName(name string) {
i.m.Lock()
defer i.m.Unlock()
i.nameMap[name] = struct{}{}
}

func (i *ignorant) RemoveName(name string) {
i.m.Lock()
defer i.m.Unlock()
delete(i.nameMap, name)
}

func (i *ignorant) ShouldIgnoreKey(req ctrl.Request) bool {
i.m.Lock()
defer i.m.Unlock()
_, ok := i.nameMap[req.Name]
return ok
}
22 changes: 15 additions & 7 deletions pkg/composed/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,18 +179,26 @@ func (s *baseState) PatchObjStatus(ctx context.Context) error {
// characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',
// or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')"}
func (s *baseState) PatchObjAddFinalizer(ctx context.Context, f string) (bool, error) {
added := controllerutil.AddFinalizer(s.Obj(), f)
return PatchObjAddFinalizer(ctx, f, s.Obj(), s.Cluster().K8sClient())
}

// PatchObjRemoveFinalizer patches obj with JSONPatchType removing the specified finalizer name
func (s *baseState) PatchObjRemoveFinalizer(ctx context.Context, f string) (bool, error) {
return PatchObjRemoveFinalizer(ctx, f, s.Obj(), s.Cluster().K8sClient())
}

func PatchObjAddFinalizer(ctx context.Context, f string, obj client.Object, clnt client.Client) (bool, error) {
added := controllerutil.AddFinalizer(obj, f)
if !added {
return false, nil
}
p := []byte(fmt.Sprintf(`{"metadata": {"finalizers":["%s"]}}`, f))
return true, s.Cluster().K8sClient().Patch(ctx, s.Obj(), client.RawPatch(types.MergePatchType, p))
return true, clnt.Patch(ctx, obj, client.RawPatch(types.MergePatchType, p))
}

// PatchObjRemoveFinalizer patches obj with JSONPatchType removing the specified finalizer name
func (s *baseState) PatchObjRemoveFinalizer(ctx context.Context, f string) (bool, error) {
func PatchObjRemoveFinalizer(ctx context.Context, f string, obj client.Object, clnt client.Client) (bool, error) {
idx := -1
for i, s := range s.Obj().GetFinalizers() {
for i, s := range obj.GetFinalizers() {
if s == f {
idx = i
break
Expand All @@ -199,7 +207,7 @@ func (s *baseState) PatchObjRemoveFinalizer(ctx context.Context, f string) (bool
if idx == -1 {
return false, nil
}
controllerutil.RemoveFinalizer(s.Obj(), f)
controllerutil.RemoveFinalizer(obj, f)
p := []byte(fmt.Sprintf(`[{"op": "remove", "path": "/metadata/finalizers/%d"}]`, idx))
return true, s.Cluster().K8sClient().Patch(ctx, s.Obj(), client.RawPatch(types.JSONPatchType, p))
return true, clnt.Patch(ctx, obj, client.RawPatch(types.JSONPatchType, p))
}
3 changes: 2 additions & 1 deletion pkg/feature/ffApiDisabled_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package feature

import (
"context"
"github.com/go-logr/logr"
"testing"

cloudresourcesv1beta1 "github.com/kyma-project/cloud-manager/api/cloud-resources/v1beta1"
Expand All @@ -17,7 +18,7 @@ func TestApiDisabled(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

err := Initialize(ctx, WithFile("testdata/apiDisabled.yaml"))
err := Initialize(ctx, logr.Discard(), WithFile("testdata/apiDisabled.yaml"))
assert.NoError(t, err)

sch := runtime.NewScheme()
Expand Down
2 changes: 1 addition & 1 deletion pkg/feature/ffGcpNfsVolumeAutomaticLocationAllocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ var GcpNfsVolumeAutomaticLocationAllocation = &gcpNfsVolumeAutomaticLocationAllo
type gcpNfsVolumeAutomaticLocationAllocationInfo struct{}

func (k *gcpNfsVolumeAutomaticLocationAllocationInfo) Value(ctx context.Context) bool {
return provider.BoolVariation(ctx, gcpNfsVolumeAutomaticLocationAllocationFlagName, false)
return provider.BoolVariation(ctx, gcpNfsVolumeAutomaticLocationAllocationFlagName, true)
}
2 changes: 1 addition & 1 deletion pkg/feature/ffIpRangeAutomaticCidrAllocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ var IpRangeAutomaticCidrAllocation = &ipRangeAutomaticCidrAllocationInfo{}
type ipRangeAutomaticCidrAllocationInfo struct{}

func (k *ipRangeAutomaticCidrAllocationInfo) Value(ctx context.Context) bool {
return provider.BoolVariation(ctx, ipRangeAutomaticCidrAllocationFlagName, false)
return provider.BoolVariation(ctx, ipRangeAutomaticCidrAllocationFlagName, true)
}
27 changes: 9 additions & 18 deletions pkg/feature/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/thomaspoignant/go-feature-flag/retriever"
"github.com/thomaspoignant/go-feature-flag/retriever/fileretriever"
"github.com/thomaspoignant/go-feature-flag/retriever/httpretriever"
"log/slog"
"os"
"time"
)
Expand All @@ -21,10 +22,8 @@ func init() {
}

type ProviderOptions struct {
logger logr.Logger
loggerSet bool
filename string
url string
filename string
url string
}

type ProviderOption func(o *ProviderOptions)
Expand All @@ -41,21 +40,12 @@ func WithUrl(url string) ProviderOption {
}
}

func WithLogger(logger logr.Logger) ProviderOption {
return func(o *ProviderOptions) {
o.logger = logger
o.loggerSet = true
}
}

func Initialize(ctx context.Context, opts ...ProviderOption) (err error) {
func Initialize(ctx context.Context, logger logr.Logger, opts ...ProviderOption) (err error) {
o := &ProviderOptions{}
for _, x := range opts {
x(o)
}
if !o.loggerSet {
o.logger = logr.Discard()
}

if len(o.filename) == 0 && len(o.url) == 0 {
o.url = os.Getenv("FEATURE_FLAG_CONFIG_URL")
if len(o.url) == 0 {
Expand All @@ -67,19 +57,20 @@ func Initialize(ctx context.Context, opts ...ProviderOption) (err error) {
}
var rtvr retriever.Retriever
if o.url != "" {
o.logger.WithValues("url", o.url).Info("Using http retriever")
logger.WithValues("ffRetrieverUrl", o.url).Info("Using http retriever")
rtvr = &httpretriever.Retriever{URL: o.url}
} else {
o.logger.WithValues("file", o.filename).Info("Using file retriever")
logger.WithValues("ffRetrieverFile", o.filename).Info("Using file retriever")
rtvr = &fileretriever.Retriever{Path: o.filename}
}
ff, errLoading := ffclient.New(ffclient.Config{
PollingInterval: 10 * time.Second,
PollingInterval: 30 * time.Second,
Context: ctx,
FileFormat: "yaml",
EnablePollingJitter: true,
StartWithRetrieverError: true,
Retriever: rtvr,
LeveledLogger: slog.New(logr.ToSlogHandler(logger)),
})
if errLoading != nil {
err = fmt.Errorf("loading error: %w", errLoading)
Expand Down
2 changes: 0 additions & 2 deletions pkg/testinfra/dsl/gcpNfsVolume.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ func CreateGcpNfsVolume(ctx context.Context, clnt client.Client, obj *cloudresou
// some error
return err
}
//write entire object on the console
fmt.Println(obj)
err = clnt.Create(ctx, obj)
return err
}
Expand Down
Loading