Skip to content

Commit 40380d6

Browse files
committed
chore: resolve review comments
1 parent 8c4fa11 commit 40380d6

File tree

2 files changed

+24
-18
lines changed

2 files changed

+24
-18
lines changed

pkg/apiserver/apiserver.go

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"k8s.io/client-go/discovery"
3737
"k8s.io/client-go/kubernetes"
3838
"k8s.io/client-go/rest"
39+
"k8s.io/client-go/util/retry"
3940
apiregv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
4041
aggregatorclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset"
4142
)
@@ -279,19 +280,22 @@ func getServerVersion(discoveryClient discovery.DiscoveryInterface) (semver.Vers
279280
}
280281

281282
func updateAPIService(ctx context.Context, logger logr.Logger, client aggregatorclient.Interface, caProvider dynamiccertificates.CAContentProvider) error {
282-
apiService, err := client.ApiregistrationV1().APIServices().Get(ctx, apiServiceName, metav1.GetOptions{})
283-
if err != nil {
284-
return fmt.Errorf("error getting APIService %s: %v", apiServiceName, err)
285-
}
283+
if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
284+
apiService, err := client.ApiregistrationV1().APIServices().Get(ctx, apiServiceName, metav1.GetOptions{})
285+
if err != nil {
286+
return fmt.Errorf("error getting APIService %s: %v", apiServiceName, err)
287+
}
286288

287-
caBundle := caProvider.CurrentCABundleContent()
288-
if bytes.Equal(apiService.Spec.CABundle, caBundle) {
289-
return nil
290-
}
289+
caBundle := caProvider.CurrentCABundleContent()
290+
if bytes.Equal(apiService.Spec.CABundle, caBundle) {
291+
return nil
292+
}
291293

292-
logger.Info("Syncing CA certificate with APIServices")
293-
apiService.Spec.CABundle = caBundle
294-
if _, err := client.ApiregistrationV1().APIServices().Update(ctx, apiService, metav1.UpdateOptions{}); err != nil {
294+
logger.Info("Syncing CA certificate with APIServices")
295+
apiService.Spec.CABundle = caBundle
296+
_, err = client.ApiregistrationV1().APIServices().Update(ctx, apiService, metav1.UpdateOptions{})
297+
return err
298+
}); err != nil {
295299
return fmt.Errorf("error updating kapp-controller CA cert of APIService %s: %v", apiServiceName, err)
296300
}
297301
return nil

pkg/apiserver/apiserver_test.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,15 @@ type fakeCAProvider struct {
2222
bundle []byte
2323
}
2424

25-
func (f *fakeCAProvider) Name() string { return "fake-ca-provider" }
26-
func (f *fakeCAProvider) CurrentCABundleContent() []byte { return f.bundle }
27-
func (f *fakeCAProvider) AddListener(listener dynamiccertificates.Listener) {}
25+
func (f *fakeCAProvider) Name() string { return "fake-ca-provider" }
26+
func (f *fakeCAProvider) CurrentCABundleContent() []byte { return f.bundle }
27+
func (f *fakeCAProvider) AddListener(_ dynamiccertificates.Listener) {}
2828
func (f *fakeCAProvider) VerifyOptions() (x509.VerifyOptions, bool) {
2929
return x509.VerifyOptions{}, false
3030
}
3131

3232
func Test_updateAPIService(t *testing.T) {
33-
apiServiceName := "v1alpha1.data.packaging.carvel.dev"
34-
dummyLogger := logr.Discard()
33+
logger := logr.Discard()
3534

3635
tests := []struct {
3736
name string
@@ -65,7 +64,7 @@ func Test_updateAPIService(t *testing.T) {
6564

6665
fakeProvider := &fakeCAProvider{bundle: tc.newBundle}
6766

68-
err := updateAPIService(context.TODO(), dummyLogger, fakeClient, fakeProvider)
67+
err := updateAPIService(context.TODO(), logger, fakeClient, fakeProvider)
6968
require.NoError(t, err)
7069

7170
actions := fakeClient.Actions()
@@ -76,7 +75,10 @@ func Test_updateAPIService(t *testing.T) {
7675
for _, action := range actions {
7776
if action.GetVerb() == "update" {
7877
updateActionFound = true
79-
updateAction := action.(clienttesting.UpdateAction)
78+
updateAction, ok := action.(clienttesting.UpdateAction)
79+
if !ok {
80+
t.Fatalf("Expected UpdateAction, got %T", action)
81+
}
8082
updatedSvc := updateAction.GetObject().(*apiregv1.APIService)
8183
require.Equal(t, tc.newBundle, updatedSvc.Spec.CABundle)
8284
}

0 commit comments

Comments
 (0)