Skip to content

Commit

Permalink
fix: forward context in ToolchainConfig reconciler (#914)
Browse files Browse the repository at this point in the history 
Signed-off-by: Francesco Ilario <filario@redhat.com>
Co-authored-by: Alexey Kazakov <alkazako@redhat.com>
  • Loading branch information
@filariow @alexeykazakov
filariow and alexeykazakov authored Nov 4, 2023
1 parent faf855a commit 523e88f
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 21 deletions.
12 changes: 6 additions & 6 deletions controllers/toolchainconfig/sync.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func NewSynchronizer(logger logr.Logger, getMembersFunc cluster.GetMemberCluster

// SyncMemberConfigs retrieves member operator configurations and syncs the appropriate configuration to each member cluster
// returns a map of any errors encountered indexed by cluster name
func (s *Synchronizer) SyncMemberConfigs(sourceConfig *toolchainv1alpha1.ToolchainConfig) map[string]string {
func (s *Synchronizer) SyncMemberConfigs(ctx context.Context, sourceConfig *toolchainv1alpha1.ToolchainConfig) map[string]string {
toolchainConfig := sourceConfig.DeepCopy()

// get configs for member toolchainclusters
Expand All @@ -50,7 +50,7 @@ func (s *Synchronizer) SyncMemberConfigs(sourceConfig *toolchainv1alpha1.Toolcha
delete(membersWithSpecificConfig, toolchainCluster.Name)
}

if err := SyncMemberConfig(memberConfigSpec, toolchainCluster); err != nil {
if err := SyncMemberConfig(ctx, memberConfigSpec, toolchainCluster); err != nil {
s.logger.Error(err, "failed to sync MemberOperatorConfig", "cluster_name", toolchainCluster.Name)
syncErrors[toolchainCluster.Name] = err.Error()
}
Expand All @@ -65,9 +65,9 @@ func (s *Synchronizer) SyncMemberConfigs(sourceConfig *toolchainv1alpha1.Toolcha
return syncErrors
}

func SyncMemberConfig(memberConfigSpec toolchainv1alpha1.MemberOperatorConfigSpec, memberCluster *cluster.CachedToolchainCluster) error {
func SyncMemberConfig(ctx context.Context, memberConfigSpec toolchainv1alpha1.MemberOperatorConfigSpec, memberCluster *cluster.CachedToolchainCluster) error {
memberConfig := &toolchainv1alpha1.MemberOperatorConfig{}
if err := memberCluster.Client.Get(context.TODO(), types.NamespacedName{Namespace: memberCluster.OperatorNamespace, Name: configResourceName}, memberConfig); err != nil {
if err := memberCluster.Client.Get(ctx, types.NamespacedName{Namespace: memberCluster.OperatorNamespace, Name: configResourceName}, memberConfig); err != nil {
if errors.IsNotFound(err) {
// MemberOperatorConfig does not exist - create it
memberConfig := &toolchainv1alpha1.MemberOperatorConfig{
Expand All @@ -77,7 +77,7 @@ func SyncMemberConfig(memberConfigSpec toolchainv1alpha1.MemberOperatorConfigSpe
},
Spec: memberConfigSpec,
}
return memberCluster.Client.Create(context.TODO(), memberConfig)
return memberCluster.Client.Create(ctx, memberConfig)
}
// Error reading the object - try again on the next reconcile
return err
Expand All @@ -86,5 +86,5 @@ func SyncMemberConfig(memberConfigSpec toolchainv1alpha1.MemberOperatorConfigSpe
// MemberOperatorConfig exists - update spec
memberConfig.Spec = memberConfigSpec

return memberCluster.Client.Update(context.TODO(), memberConfig)
return memberCluster.Client.Update(ctx, memberConfig)
}
28 changes: 14 additions & 14 deletions controllers/toolchainconfig/sync_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestSyncMemberConfigs(t *testing.T) {
)

// when
syncErrors := s.SyncMemberConfigs(toolchainConfig)
syncErrors := s.SyncMemberConfigs(context.TODO(), toolchainConfig)

// then
require.Empty(t, syncErrors)
Expand All @@ -54,7 +54,7 @@ func TestSyncMemberConfigs(t *testing.T) {
)

// when
syncErrors := s.SyncMemberConfigs(toolchainConfig)
syncErrors := s.SyncMemberConfigs(context.TODO(), toolchainConfig)

// then
require.Empty(t, syncErrors)
Expand All @@ -66,7 +66,7 @@ func TestSyncMemberConfigs(t *testing.T) {
t.Run("sync to a member failed", func(t *testing.T) {
// given
memberCl := test.NewFakeClient(t)
memberCl.MockGet = func(ctx context.Context, key types.NamespacedName, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
memberCl.MockGet = func(_ context.Context, key types.NamespacedName, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
return fmt.Errorf("client error")
}
toolchainConfig := commonconfig.NewToolchainConfigObjWithReset(t,
Expand All @@ -78,7 +78,7 @@ func TestSyncMemberConfigs(t *testing.T) {
)

// when
syncErrors := s.SyncMemberConfigs(toolchainConfig)
syncErrors := s.SyncMemberConfigs(context.TODO(), toolchainConfig)

// then
require.Len(t, syncErrors, 1)
Expand All @@ -88,11 +88,11 @@ func TestSyncMemberConfigs(t *testing.T) {
t.Run("sync to multiple members failed", func(t *testing.T) {
// given
memberCl := test.NewFakeClient(t)
memberCl.MockGet = func(ctx context.Context, key types.NamespacedName, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
memberCl.MockGet = func(_ context.Context, key types.NamespacedName, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
return fmt.Errorf("client error")
}
memberCl2 := test.NewFakeClient(t)
memberCl2.MockGet = func(ctx context.Context, key types.NamespacedName, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
memberCl2.MockGet = func(_ context.Context, key types.NamespacedName, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
return fmt.Errorf("client2 error")
}
toolchainConfig := commonconfig.NewToolchainConfigObjWithReset(t,
Expand All @@ -104,7 +104,7 @@ func TestSyncMemberConfigs(t *testing.T) {
)

// when
syncErrors := s.SyncMemberConfigs(toolchainConfig)
syncErrors := s.SyncMemberConfigs(context.TODO(), toolchainConfig)

// then
require.Len(t, syncErrors, 2)
Expand All @@ -124,7 +124,7 @@ func TestSyncMemberConfigs(t *testing.T) {
)

// when
syncErrors := s.SyncMemberConfigs(toolchainConfig)
syncErrors := s.SyncMemberConfigs(context.TODO(), toolchainConfig)

// then
require.Len(t, syncErrors, 1)
Expand All @@ -143,7 +143,7 @@ func TestSyncMemberConfig(t *testing.T) {
memberConfig := testconfig.NewMemberOperatorConfigObj(testconfig.MemberStatus().RefreshPeriod("5s"))

// when
err := toolchainconfig.SyncMemberConfig(memberConfig.Spec, memberCluster)
err := toolchainconfig.SyncMemberConfig(context.TODO(), memberConfig.Spec, memberCluster)

// then
require.NoError(t, err)
Expand All @@ -161,7 +161,7 @@ func TestSyncMemberConfig(t *testing.T) {
memberConfig := testconfig.NewMemberOperatorConfigObj(testconfig.MemberStatus().RefreshPeriod("5s"))

// when
err := toolchainconfig.SyncMemberConfig(memberConfig.Spec, memberCluster)
err := toolchainconfig.SyncMemberConfig(context.TODO(), memberConfig.Spec, memberCluster)

// then
require.NoError(t, err)
Expand All @@ -176,14 +176,14 @@ func TestSyncMemberConfig(t *testing.T) {
t.Run("client get error", func(t *testing.T) {
// given
memberCl := test.NewFakeClient(t)
memberCl.MockGet = func(ctx context.Context, key types.NamespacedName, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
memberCl.MockGet = func(_ context.Context, key types.NamespacedName, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
return fmt.Errorf("client error")
}
memberCluster := NewMemberClusterWithClient(memberCl, "member1", corev1.ConditionTrue)
memberConfig := testconfig.NewMemberOperatorConfigObj(testconfig.MemberStatus().RefreshPeriod("5s"))

// when
err := toolchainconfig.SyncMemberConfig(memberConfig.Spec, memberCluster)
err := toolchainconfig.SyncMemberConfig(context.TODO(), memberConfig.Spec, memberCluster)

// then
require.EqualError(t, err, "client error")
Expand All @@ -196,14 +196,14 @@ func TestSyncMemberConfig(t *testing.T) {
// given
originalConfig := testconfig.NewMemberOperatorConfigObj(testconfig.MemberStatus().RefreshPeriod("10s"))
memberCl := test.NewFakeClient(t, originalConfig)
memberCl.MockUpdate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.UpdateOption) error {
memberCl.MockUpdate = func(_ context.Context, obj runtimeclient.Object, opts ...runtimeclient.UpdateOption) error {
return fmt.Errorf("client update error")
}
memberCluster := NewMemberClusterWithClient(memberCl, "member1", corev1.ConditionTrue)
memberConfig := testconfig.NewMemberOperatorConfigObj(testconfig.MemberStatus().RefreshPeriod("5s"))

// when
err := toolchainconfig.SyncMemberConfig(memberConfig.Spec, memberCluster)
err := toolchainconfig.SyncMemberConfig(context.TODO(), memberConfig.Spec, memberCluster)

// then
require.EqualError(t, err, "client update error")
Expand Down
2 changes: 1 addition & 1 deletion controllers/toolchainconfig/toolchainconfig_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
getMembersFunc: r.GetMembersFunc,
}

if syncErrs := sync.SyncMemberConfigs(toolchainConfig); len(syncErrs) > 0 {
if syncErrs := sync.SyncMemberConfigs(ctx, toolchainConfig); len(syncErrs) > 0 {
for cluster, errMsg := range syncErrs {
err := fmt.Errorf(errMsg)
reqLogger.Error(err, "error syncing configuration to member cluster", "cluster", cluster)
Expand Down

0 comments on commit 523e88f

Please sign in to comment.