Skip to content

Commit

Permalink
- Ensure a unique application is found (findApplicationByName)
Browse files Browse the repository at this point in the history 
- Error wrapping for improved reporting in higher-lvl code
- Change UpdateSpec retry to stop trying, with exponential backoff
- Add and enhance tests. Improve code coverage.

Signed-off-by: Jort Koopmans <[email protected]>
  • Loading branch information
@jortkoopmans
jortkoopmans committed Sep 18, 2024
1 parent d440337 commit d4b988f
Show file tree
Hide file tree
Showing 2 changed files with 192 additions and 40 deletions.
57 changes: 39 additions & 18 deletions pkg/argocd/argocd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"path/filepath"
"strings"
"time"

"github.com/argoproj-labs/argocd-image-updater/pkg/common"
"github.com/argoproj-labs/argocd-image-updater/pkg/image"
Expand All @@ -32,67 +33,87 @@ func (client *k8sClient) GetApplication(ctx context.Context, appName string) (*v
// List all applications across all namespaces (using empty labelSelector)
appList, err := client.ListApplications("")
if err != nil {
log.Errorf("Error listing applications: %v", err)
return nil, err
return nil, fmt.Errorf("error listing applications: %w", err)
}

// Filter applications by name using nameMatchesPattern
app, err := findApplicationByName(appList, appName)
if err != nil {
log.Errorf("Error getting application: %v", err)
return nil, err
log.Errorf("error getting application: %v", err)
return nil, fmt.Errorf("error getting application: %w", err)
}

// Retrieve the application in the specified namespace
return client.kubeClient.ApplicationsClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(ctx, appName, v1.GetOptions{})
return client.kubeClient.ApplicationsClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(ctx, app.Name, v1.GetOptions{})
}

// ListApplications lists all applications across all namespaces.
func (client *k8sClient) ListApplications(labelSelector string) ([]v1alpha1.Application, error) {

list, err := client.kubeClient.ApplicationsClientset.ArgoprojV1alpha1().Applications(v1.NamespaceAll).List(context.TODO(), v1.ListOptions{LabelSelector: labelSelector})
if err != nil {
log.Errorf("Error listing applications: %v", err)
return nil, err
return nil, fmt.Errorf("error listing applications: %w", err)
}

log.Debugf("Applications listed: %d", len(list.Items))
return list.Items, nil
}

// findApplicationByName filters the list of applications by name using nameMatchesPattern.
func findApplicationByName(appList []v1alpha1.Application, appName string) (*v1alpha1.Application, error) {
var matchedApps []*v1alpha1.Application

for _, app := range appList {
log.Debugf("Found application: %s in namespace %s", app.Name, app.Namespace)
if nameMatchesPattern(app.Name, []string{appName}) {
log.Debugf("Successfully found application: %s", app.Name)
return &app, nil
log.Debugf("Application %s matches the pattern", app.Name)
matchedApps = append(matchedApps, &app)
}
}

// If the application is not found, return an error
err := errors.NewNotFound(v1alpha1.Resource("application"), appName)
return nil, err
if len(matchedApps) == 0 {
return nil, fmt.Errorf("application %s not found", appName)
}

if len(matchedApps) > 1 {
return nil, fmt.Errorf("multiple applications found matching %s", appName)
}

return matchedApps[0], nil
}

func (client *k8sClient) UpdateSpec(ctx context.Context, spec *application.ApplicationUpdateSpecRequest) (*v1alpha1.ApplicationSpec, error) {
for {
const defaultMaxRetries = 7
const baseDelay = 100 * time.Millisecond // Initial delay before retrying

// Allow overriding max retries for testing purposes
maxRetries := defaultMaxRetries
if overrideRetries, ok := os.LookupEnv("OVERRIDE_MAX_RETRIES"); ok {
var retries int
if _, err := fmt.Sscanf(overrideRetries, "%d", &retries); err == nil {
maxRetries = retries
}
}

for attempts := 0; attempts < maxRetries; attempts++ {
app, err := client.GetApplication(ctx, spec.GetName())
if err != nil {
log.Errorf("Could not get Application: %s", spec.GetName())
return nil, err
log.Errorf("could not get application: %s, error: %v", spec.GetName(), err)
return nil, fmt.Errorf("error getting application: %w", err)
}
app.Spec = *spec.Spec

updatedApp, err := client.kubeClient.ApplicationsClientset.ArgoprojV1alpha1().Applications(app.Namespace).Update(ctx, app, v1.UpdateOptions{})
if err != nil {
if errors.IsConflict(err) {
log.Warnf("conflict occurred while updating application: %s, retrying... (%d/%d)", spec.GetName(), attempts+1, maxRetries)
time.Sleep(baseDelay * (1 << attempts)) // Exponential backoff, multiply baseDelay by 2^attempts
continue
}
return nil, err
log.Errorf("could not update application: %s, error: %v", spec.GetName(), err)
return nil, fmt.Errorf("error updating application: %w", err)
}
return &updatedApp.Spec, nil
}
return nil, fmt.Errorf("max retries(%d) reached while updating application: %s", maxRetries, spec.GetName())
}

// NewK8SClient creates a new kubernetes client to interact with kubernetes api-server.
Expand Down
175 changes: 153 additions & 22 deletions pkg/argocd/argocd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package argocd
import (
"context"
"fmt"
"os"
"testing"

"github.com/argoproj-labs/argocd-image-updater/pkg/common"
Expand Down Expand Up @@ -1043,44 +1044,174 @@ func TestKubernetesClient(t *testing.T) {
t.Run("Get application not found", func(t *testing.T) {
_, err := client.GetApplication(context.TODO(), "test-app-non-existent")
require.Error(t, err)
assert.True(t, errors.IsNotFound(err))
assert.Contains(t, err.Error(), "application test-app-non-existent not found")
})

t.Run("List and Get applications errors", func(t *testing.T) {
// Create a fake clientset
clientset := fake.NewSimpleClientset()

// Simulate an error in the List action
clientset.PrependReactor("list", "applications", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, errors.NewInternalError(fmt.Errorf("list error"))
})

// Create the Kubernetes client
client, err := NewK8SClient(&kube.KubernetesClient{
ApplicationsClientset: clientset,
})
require.NoError(t, err)

// Test ListApplications error handling
apps, err := client.ListApplications("")
assert.Nil(t, apps)
assert.EqualError(t, err, "error listing applications: Internal error occurred: list error")

// Test GetApplication error handling
_, err = client.GetApplication(context.TODO(), "test-app")
assert.Error(t, err)
assert.Contains(t, err.Error(), "error listing applications: Internal error occurred: list error")
})

t.Run("Get applications with multiple applications found", func(t *testing.T) {
// Create a fake clientset with multiple applications having the same name
clientset := fake.NewSimpleClientset(
&v1alpha1.Application{
ObjectMeta: v1.ObjectMeta{Name: "test-app", Namespace: "ns1"},
Spec: v1alpha1.ApplicationSpec{},
},
&v1alpha1.Application{
ObjectMeta: v1.ObjectMeta{Name: "test-app", Namespace: "ns2"},
Spec: v1alpha1.ApplicationSpec{},
},
)

// Create the Kubernetes client
client, err := NewK8SClient(&kube.KubernetesClient{
ApplicationsClientset: clientset,
})
require.NoError(t, err)

// Test GetApplication with multiple matching applications
_, err = client.GetApplication(context.TODO(), "test-app")
assert.Error(t, err)
assert.EqualError(t, err, "error getting application: multiple applications found matching test-app")
})
}

func TestKubernetesClient_UpdateSpec_Conflict(t *testing.T) {
func TestKubernetesClientUpdateSpec(t *testing.T) {
app := &v1alpha1.Application{
ObjectMeta: v1.ObjectMeta{Name: "test-app", Namespace: "testns"},
}
clientset := fake.NewSimpleClientset(app)

attempts := 0
clientset.PrependReactor("update", "*", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
if attempts == 0 {
attempts++
return true, nil, errors.NewConflict(
schema.GroupResource{Group: "argoproj.io", Resource: "Application"}, app.Name, fmt.Errorf("conflict updating %s", app.Name))
} else {
return false, nil, nil
}
t.Run("Successful update after conflict retry", func(t *testing.T) {
attempts := 0
clientset.PrependReactor("update", "*", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
if attempts == 0 {
attempts++
return true, nil, errors.NewConflict(
schema.GroupResource{Group: "argoproj.io", Resource: "Application"}, app.Name, fmt.Errorf("conflict updating %s", app.Name))
} else {
return false, nil, nil
}
})

client, err := NewK8SClient(&kube.KubernetesClient{
ApplicationsClientset: clientset,
})
require.NoError(t, err)

appName := "test-app"
spec, err := client.UpdateSpec(context.TODO(), &application.ApplicationUpdateSpecRequest{
Name: &appName,
Spec: &v1alpha1.ApplicationSpec{Source: &v1alpha1.ApplicationSource{
RepoURL: "https://github.com/argoproj/argocd-example-apps",
}},
})

require.NoError(t, err)
assert.Equal(t, "https://github.com/argoproj/argocd-example-apps", spec.Source.RepoURL)
})

client, err := NewK8SClient(&kube.KubernetesClient{
ApplicationsClientset: clientset,
t.Run("UpdateSpec errors - application not found", func(t *testing.T) {
// Create a fake empty clientset
clientset := fake.NewSimpleClientset()

client, err := NewK8SClient(&kube.KubernetesClient{
ApplicationsClientset: clientset,
})
require.NoError(t, err)

appName := "test-app"
appNamespace := "testns"
spec := &application.ApplicationUpdateSpecRequest{
Name: &appName,
AppNamespace: &appNamespace,
Spec: &v1alpha1.ApplicationSpec{},
}

_, err = client.UpdateSpec(context.TODO(), spec)
assert.Error(t, err)
assert.Contains(t, err.Error(), "error getting application: application test-app not found")
})
require.NoError(t, err)

appName := "test-app"
t.Run("UpdateSpec errors - conflict failing retries", func(t *testing.T) {
clientset := fake.NewSimpleClientset(&v1alpha1.Application{
ObjectMeta: v1.ObjectMeta{Name: "test-app", Namespace: "testns"},
Spec: v1alpha1.ApplicationSpec{},
})

client, err := NewK8SClient(&kube.KubernetesClient{
ApplicationsClientset: clientset,
})
require.NoError(t, err)

clientset.PrependReactor("update", "applications", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, errors.NewConflict(v1alpha1.Resource("applications"), "test-app", fmt.Errorf("conflict error"))
})

spec, err := client.UpdateSpec(context.TODO(), &application.ApplicationUpdateSpecRequest{
Name: &appName,
Spec: &v1alpha1.ApplicationSpec{Source: &v1alpha1.ApplicationSource{
RepoURL: "https://github.com/argoproj/argocd-example-apps",
}},
os.Setenv("OVERRIDE_MAX_RETRIES", "0")
defer os.Unsetenv("OVERRIDE_MAX_RETRIES")

appName := "test-app"
spec := &application.ApplicationUpdateSpecRequest{
Name: &appName,
Spec: &v1alpha1.ApplicationSpec{},
}

_, err = client.UpdateSpec(context.TODO(), spec)
assert.Error(t, err)
assert.Contains(t, err.Error(), "max retries(0) reached while updating application: test-app")
})

require.NoError(t, err)
t.Run("UpdateSpec errors - non-conflict update error", func(t *testing.T) {
clientset := fake.NewSimpleClientset(&v1alpha1.Application{
ObjectMeta: v1.ObjectMeta{Name: "test-app", Namespace: "testns"},
Spec: v1alpha1.ApplicationSpec{},
})

assert.Equal(t, "https://github.com/argoproj/argocd-example-apps", spec.Source.RepoURL)
client, err := NewK8SClient(&kube.KubernetesClient{
ApplicationsClientset: clientset,
})
require.NoError(t, err)

clientset.PrependReactor("update", "applications", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, fmt.Errorf("non-conflict error")
})

appName := "test-app"
appNamespace := "testns"
spec := &application.ApplicationUpdateSpecRequest{
Name: &appName,
AppNamespace: &appNamespace,
Spec: &v1alpha1.ApplicationSpec{},
}

_, err = client.UpdateSpec(context.TODO(), spec)
assert.Error(t, err)
assert.Contains(t, err.Error(), "error updating application: non-conflict error")
})
}

func Test_parseImageList(t *testing.T) {
Expand Down

0 comments on commit d4b988f

Please sign in to comment.