Skip to content

Commit d104052

Browse files
janiskemperjaniskemper
authored
Merge pull request #187 from SovereignCloudStack/tg/create-generic-client-to-download-release-assets-via-oci
🌱 Create a Generic client to download release assets
2 parents 04bce41 + 454b031 commit d104052

File tree

23 files changed

+437
-326
lines changed

23 files changed

+437
-326
lines changed

‎api/v1alpha1/conditions_const.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,17 @@ const (
7878
)
7979

8080
const (
81-
// GitAPIAvailableCondition is used when Git API is available.
82-
GitAPIAvailableCondition clusterv1.ConditionType = "GitAPIAvailable"
81+
// AssetsClientAPIAvailableCondition is used when AssetsClient API is available.
82+
AssetsClientAPIAvailableCondition clusterv1.ConditionType = "AssetsClientAPIAvailable"
8383

84-
// GitTokenOrEnvVariableNotSetReason is used when user don't specify the token or environment variable.
85-
GitTokenOrEnvVariableNotSetReason = "GitTokenOrEnvVariableNotSet" //#nosec
84+
// FailedCreateAssetsClientReason is used when user don't specify the token or environment variable required for initializing the assets client.
85+
FailedCreateAssetsClientReason = "FailedCreateAssetsClient" //#nosec
8686
)
8787

8888
const (
89-
// GitReleasesSyncedCondition is used when Git releases have been synced successfully.
90-
GitReleasesSyncedCondition clusterv1.ConditionType = "GitReleasesSynced"
89+
// ReleasesSyncedCondition is used when releases have been synced successfully.
90+
ReleasesSyncedCondition clusterv1.ConditionType = "ReleasesSynced"
9191

92-
// FailedToSyncReason is used when Git releases could not be synced.
92+
// FailedToSyncReason is used when releases could not be synced.
9393
FailedToSyncReason = "FailedToSync"
9494
)

‎cmd/main.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,10 @@ import (
2727
//+kubebuilder:scaffold:imports
2828
csov1alpha1 "github.com/SovereignCloudStack/cluster-stack-operator/api/v1alpha1"
2929
"github.com/SovereignCloudStack/cluster-stack-operator/internal/controller"
30+
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient"
31+
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient/fake"
32+
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient/github"
3033
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/csoversion"
31-
githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client"
32-
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client/fake"
3334
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/kube"
3435
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/utillog"
3536
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/workloadcluster"
@@ -131,11 +132,11 @@ func main() {
131132
// Setup the context that's going to be used in controllers and for the manager.
132133
ctx := ctrl.SetupSignalHandler()
133134

134-
var gitFactory githubclient.Factory
135+
var assetsClientFactory assetsclient.Factory
135136
if localMode {
136-
gitFactory = fake.NewFactory()
137+
assetsClientFactory = fake.NewFactory()
137138
} else {
138-
gitFactory = githubclient.NewFactory()
139+
assetsClientFactory = github.NewFactory()
139140
}
140141

141142
restConfig := mgr.GetConfig()
@@ -153,7 +154,7 @@ func main() {
153154
if err = (&controller.ClusterStackReconciler{
154155
Client: mgr.GetClient(),
155156
ReleaseDirectory: releaseDir,
156-
GitHubClientFactory: gitFactory,
157+
AssetsClientFactory: assetsClientFactory,
157158
WatchFilterValue: watchFilterValue,
158159
}).SetupWithManager(ctx, mgr, controllerruntimecontroller.Options{MaxConcurrentReconciles: clusterStackConcurrency}); err != nil {
159160
setupLog.Error(err, "unable to create controller", "controller", "ClusterStack")
@@ -166,7 +167,7 @@ func main() {
166167
ReleaseDirectory: releaseDir,
167168
WatchFilterValue: watchFilterValue,
168169
KubeClientFactory: kube.NewFactory(),
169-
GitHubClientFactory: gitFactory,
170+
AssetsClientFactory: assetsClientFactory,
170171
}).SetupWithManager(ctx, mgr, controllerruntimecontroller.Options{MaxConcurrentReconciles: clusterStackReleaseConcurrency}); err != nil {
171172
setupLog.Error(err, "unable to create controller", "controller", "ClusterStackRelease")
172173
os.Exit(1)
@@ -179,6 +180,7 @@ func main() {
179180
WatchFilterValue: watchFilterValue,
180181
KubeClientFactory: kube.NewFactory(),
181182
WorkloadClusterFactory: workloadcluster.NewFactory(),
183+
AssetsClientFactory: assetsClientFactory,
182184
}).SetupWithManager(ctx, mgr, controllerruntimecontroller.Options{MaxConcurrentReconciles: clusterAddonConcurrency}); err != nil {
183185
setupLog.Error(err, "unable to create controller", "controller", "ClusterAddon")
184186
os.Exit(1)

‎internal/controller/clusteraddon_controller.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"time"
2828

2929
csov1alpha1 "github.com/SovereignCloudStack/cluster-stack-operator/api/v1alpha1"
30+
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient"
3031
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/kube"
3132
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/release"
3233
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/workloadcluster"
@@ -67,6 +68,7 @@ type ClusterAddonReconciler struct {
6768
RestConfigSettings
6869
ReleaseDirectory string
6970
KubeClientFactory kube.Factory
71+
AssetsClientFactory assetsclient.Factory
7072
WatchFilterValue string
7173
WorkloadClusterFactory workloadcluster.Factory
7274
}

‎internal/controller/clusterstack_controller.go

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,12 @@ import (
2323
"reflect"
2424
"sort"
2525
"strings"
26+
"time"
2627

2728
csov1alpha1 "github.com/SovereignCloudStack/cluster-stack-operator/api/v1alpha1"
2829
"github.com/SovereignCloudStack/cluster-stack-operator/internal/clusterstackrelease"
30+
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient"
2931
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/clusterstack"
30-
githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client"
3132
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/version"
3233
corev1 "k8s.io/api/core/v1"
3334
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -55,7 +56,7 @@ import (
5556
// ClusterStackReconciler reconciles a ClusterStack object.
5657
type ClusterStackReconciler struct {
5758
client.Client
58-
GitHubClientFactory githubclient.Factory
59+
AssetsClientFactory assetsclient.Factory
5960
ReleaseDirectory string
6061
WatchFilterValue string
6162
}
@@ -112,32 +113,39 @@ func (r *ClusterStackReconciler) Reconcile(ctx context.Context, req reconcile.Re
112113
var latest *string
113114

114115
if clusterStack.Spec.AutoSubscribe {
115-
gc, err := r.GitHubClientFactory.NewClient(ctx)
116+
gc, err := r.AssetsClientFactory.NewClient(ctx)
116117
if err != nil {
118+
isSet := conditions.IsFalse(clusterStack, csov1alpha1.AssetsClientAPIAvailableCondition)
117119
conditions.MarkFalse(clusterStack,
118-
csov1alpha1.GitAPIAvailableCondition,
119-
csov1alpha1.GitTokenOrEnvVariableNotSetReason,
120+
csov1alpha1.AssetsClientAPIAvailableCondition,
121+
csov1alpha1.FailedCreateAssetsClientReason,
120122
clusterv1.ConditionSeverityError,
121123
err.Error(),
122124
)
123-
record.Warnf(clusterStack, "GitTokenOrEnvVariableNotSet", err.Error())
124-
return reconcile.Result{}, fmt.Errorf("failed to create Github client: %w", err)
125+
record.Warnf(clusterStack, "FailedCreateAssetsClient", err.Error())
126+
127+
// give the assets client a second change
128+
if isSet {
129+
return reconcile.Result{RequeueAfter: 5 * time.Second}, nil
130+
}
131+
return reconcile.Result{}, nil
125132
}
126133

127-
conditions.MarkTrue(clusterStack, csov1alpha1.GitAPIAvailableCondition)
134+
conditions.MarkTrue(clusterStack, csov1alpha1.AssetsClientAPIAvailableCondition)
135+
128136
latest, err = getLatestReleaseFromRemoteRepository(ctx, clusterStack, gc)
129137
if err != nil {
130138
// only log error and mark condition as false, but continue
131139
conditions.MarkFalse(clusterStack,
132-
csov1alpha1.GitReleasesSyncedCondition,
140+
csov1alpha1.ReleasesSyncedCondition,
133141
csov1alpha1.FailedToSyncReason,
134142
clusterv1.ConditionSeverityWarning,
135143
err.Error(),
136144
)
137145
logger.Error(err, "failed to get latest release from remote repository")
138146
}
139147

140-
conditions.MarkTrue(clusterStack, csov1alpha1.GitReleasesSyncedCondition)
148+
conditions.MarkTrue(clusterStack, csov1alpha1.ReleasesSyncedCondition)
141149
}
142150

143151
inUse, err := r.getClusterStackReleasesInUse(ctx, req.Namespace)
@@ -536,21 +544,18 @@ func getLatestReadyClusterStackRelease(clusterStackReleases []*csov1alpha1.Clust
536544
return latest, k8sversion, nil
537545
}
538546

539-
func getLatestReleaseFromRemoteRepository(ctx context.Context, clusterStack *csov1alpha1.ClusterStack, gc githubclient.Client) (*string, error) {
540-
ghReleases, resp, err := gc.ListRelease(ctx)
547+
func getLatestReleaseFromRemoteRepository(ctx context.Context, clusterStack *csov1alpha1.ClusterStack, ac assetsclient.Client) (*string, error) {
548+
releases, err := ac.ListRelease(ctx)
541549
if err != nil {
542-
return nil, fmt.Errorf("failed to list releases on remote Git repository: %w", err)
543-
}
544-
if resp != nil && resp.StatusCode != 200 {
545-
return nil, fmt.Errorf("got unexpected status from call to remote Git repository: %s", resp.Status)
550+
return nil, fmt.Errorf("failed to list releases on remote repository: %w", err)
546551
}
547552

548553
var clusterStacks clusterstack.ClusterStacks
549554

550-
for _, ghRelease := range ghReleases {
551-
clusterStackObject, matches, err := matchesSpec(ghRelease.GetTagName(), &clusterStack.Spec)
555+
for _, release := range releases {
556+
clusterStackObject, matches, err := matchesSpec(release, &clusterStack.Spec)
552557
if err != nil {
553-
return nil, fmt.Errorf("failed to get match release tag %q with spec of ClusterStack: %w", ghRelease.GetTagName(), err)
558+
return nil, fmt.Errorf("failed to get match release tag %q with spec of ClusterStack: %w", release, err)
554559
}
555560

556561
if matches {

‎internal/controller/clusterstackrelease_controller.go

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"bytes"
2121
"context"
2222
"fmt"
23-
"net/http"
2423
"os"
2524
"os/exec"
2625
"path/filepath"
@@ -29,8 +28,8 @@ import (
2928
"time"
3029

3130
csov1alpha1 "github.com/SovereignCloudStack/cluster-stack-operator/api/v1alpha1"
31+
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient"
3232
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/clusterstack"
33-
githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client"
3433
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/kube"
3534
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/release"
3635
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -57,7 +56,7 @@ type ClusterStackReleaseReconciler struct {
5756
RESTConfig *rest.Config
5857
ReleaseDirectory string
5958
KubeClientFactory kube.Factory
60-
GitHubClientFactory githubclient.Factory
59+
AssetsClientFactory assetsclient.Factory
6160
externalTracker external.ObjectTracker
6261
clusterStackRelDownloadDirectoryMutex sync.Mutex
6362
WatchFilterValue string
@@ -116,27 +115,32 @@ func (r *ClusterStackReleaseReconciler) Reconcile(ctx context.Context, req recon
116115
if download {
117116
conditions.MarkFalse(clusterStackRelease, csov1alpha1.ClusterStackReleaseAssetsReadyCondition, csov1alpha1.ReleaseAssetsNotDownloadedYetReason, clusterv1.ConditionSeverityInfo, "assets not downloaded yet")
118117

119-
// this is the point where we download the release from github
118+
// this is the point where we download the release
120119
// acquire lock so that only one reconcile loop can download the release
121120
r.clusterStackRelDownloadDirectoryMutex.Lock()
122-
123121
defer r.clusterStackRelDownloadDirectoryMutex.Unlock()
124122

125-
gc, err := r.GitHubClientFactory.NewClient(ctx)
123+
ac, err := r.AssetsClientFactory.NewClient(ctx)
126124
if err != nil {
125+
isSet := conditions.IsFalse(clusterStackRelease, csov1alpha1.AssetsClientAPIAvailableCondition)
127126
conditions.MarkFalse(clusterStackRelease,
128-
csov1alpha1.GitAPIAvailableCondition,
129-
csov1alpha1.GitTokenOrEnvVariableNotSetReason,
127+
csov1alpha1.AssetsClientAPIAvailableCondition,
128+
csov1alpha1.FailedCreateAssetsClientReason,
130129
clusterv1.ConditionSeverityError,
131130
err.Error(),
132131
)
133-
record.Warnf(clusterStackRelease, "GitTokenOrEnvVariableNotSet", err.Error())
134-
return reconcile.Result{}, fmt.Errorf("failed to create Github client: %w", err)
132+
record.Warnf(clusterStackRelease, "FailedCreateAssetsClient", err.Error())
133+
134+
// give the assets client a second change
135+
if isSet {
136+
return reconcile.Result{RequeueAfter: 5 * time.Second}, nil
137+
}
138+
return reconcile.Result{}, nil
135139
}
136140

137-
conditions.MarkTrue(clusterStackRelease, csov1alpha1.GitAPIAvailableCondition)
141+
conditions.MarkTrue(clusterStackRelease, csov1alpha1.AssetsClientAPIAvailableCondition)
138142

139-
if err := downloadReleaseAssets(ctx, releaseTag, releaseAssets.LocalDownloadPath, gc); err != nil {
143+
if err := downloadReleaseAssets(ctx, releaseTag, releaseAssets.LocalDownloadPath, ac); err != nil {
140144
return reconcile.Result{}, fmt.Errorf("failed to download release assets: %w", err)
141145
}
142146

@@ -215,18 +219,8 @@ func (r *ClusterStackReleaseReconciler) reconcileDelete(ctx context.Context, rel
215219
return reconcile.Result{}, nil
216220
}
217221

218-
func downloadReleaseAssets(ctx context.Context, releaseTag, downloadPath string, gc githubclient.Client) error {
219-
repoRelease, resp, err := gc.GetReleaseByTag(ctx, releaseTag)
220-
if err != nil {
221-
return fmt.Errorf("failed to fetch release tag %q: %w", releaseTag, err)
222-
}
223-
if resp.StatusCode != http.StatusOK {
224-
return fmt.Errorf("failed to fetch release tag %s with status code %d", releaseTag, resp.StatusCode)
225-
}
226-
227-
assetlist := []string{"metadata.yaml", "cluster-addon-values.yaml", "cluster-addon", "cluster-class"}
228-
229-
if err := gc.DownloadReleaseAssets(ctx, repoRelease, downloadPath, assetlist); err != nil {
222+
func downloadReleaseAssets(ctx context.Context, releaseTag, downloadPath string, ac assetsclient.Client) error {
223+
if err := ac.DownloadReleaseAssets(ctx, releaseTag, downloadPath); err != nil {
230224
// if download failed for some reason, delete the release directory so that it can be retried in the next reconciliation
231225
if err := os.RemoveAll(downloadPath); err != nil {
232226
return fmt.Errorf("failed to remove release: %w", err)

‎internal/controller/controller_suite_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,14 @@ var _ = BeforeSuite(func() {
5858
Expect((&ClusterStackReconciler{
5959
Client: testEnv.Manager.GetClient(),
6060
ReleaseDirectory: "./../../test/releases",
61-
GitHubClientFactory: testEnv.GitHubClientFactory,
61+
AssetsClientFactory: testEnv.AssetsClientFactory,
6262
}).SetupWithManager(ctx, testEnv.Manager, c.Options{})).To(Succeed())
6363

6464
Expect((&ClusterStackReleaseReconciler{
6565
Client: testEnv.Manager.GetClient(),
6666
RESTConfig: testEnv.Manager.GetConfig(),
6767
KubeClientFactory: kube.NewFactory(),
68-
GitHubClientFactory: testEnv.GitHubClientFactory,
68+
AssetsClientFactory: testEnv.AssetsClientFactory,
6969
ReleaseDirectory: "./../../test/releases",
7070
}).SetupWithManager(ctx, testEnv.Manager, c.Options{})).To(Succeed())
7171

‎internal/test/helpers/envtest.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ import (
3131

3232
csov1alpha1 "github.com/SovereignCloudStack/cluster-stack-operator/api/v1alpha1"
3333
"github.com/SovereignCloudStack/cluster-stack-operator/internal/test/helpers/builder"
34-
githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client"
35-
githubmocks "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client/mocks"
34+
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient"
35+
assetsclientmocks "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient/mocks"
3636
kubeclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/kube"
3737
kubemocks "github.com/SovereignCloudStack/cluster-stack-operator/pkg/kube/mocks"
3838
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/test/utils"
@@ -147,9 +147,9 @@ type (
147147
cancel context.CancelFunc
148148
kind *kind.Provider
149149
WorkloadClusterClient *kubernetes.Clientset
150-
GitHubClientFactory githubclient.Factory
150+
AssetsClientFactory assetsclient.Factory
151151
KubeClientFactory kubeclient.Factory
152-
GitHubClient *githubmocks.Client
152+
AssetsClient *assetsclientmocks.Client
153153
KubeClient *kubemocks.Client
154154
}
155155
)
@@ -203,16 +203,16 @@ func NewTestEnvironment() *TestEnvironment {
203203
klog.Fatalf("unable to create manager pod namespace: %s", err)
204204
}
205205

206-
githubClient := &githubmocks.Client{}
206+
assetsClient := &assetsclientmocks.Client{}
207207
kubeClient := &kubemocks.Client{}
208208

209209
testEnv := &TestEnvironment{
210210
Manager: mgr,
211211
Client: mgr.GetClient(),
212212
Config: mgr.GetConfig(),
213-
GitHubClientFactory: githubmocks.NewGitHubFactory(githubClient),
213+
AssetsClientFactory: assetsclientmocks.NewAssetsClientFactory(assetsClient),
214214
KubeClientFactory: kubemocks.NewKubeFactory(kubeClient),
215-
GitHubClient: githubClient,
215+
AssetsClient: assetsClient,
216216
KubeClient: kubeClient,
217217
}
218218

‎internal/test/integration/github/integration_suite_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020

2121
"github.com/SovereignCloudStack/cluster-stack-operator/internal/controller"
2222
"github.com/SovereignCloudStack/cluster-stack-operator/internal/test/helpers"
23-
githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client"
23+
githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient/github"
2424
"github.com/SovereignCloudStack/cluster-stack-operator/pkg/kube"
2525
. "github.com/onsi/ginkgo/v2"
2626
. "github.com/onsi/gomega"
@@ -47,14 +47,14 @@ var _ = BeforeSuite(func() {
4747
testEnv = helpers.NewTestEnvironment()
4848
Expect((&controller.ClusterStackReconciler{
4949
Client: testEnv.Manager.GetClient(),
50-
GitHubClientFactory: githubclient.NewFactory(),
50+
AssetsClientFactory: githubclient.NewFactory(),
5151
ReleaseDirectory: "/tmp/downloads",
5252
}).SetupWithManager(ctx, testEnv.Manager, controllerruntimecontroller.Options{})).To(Succeed())
5353
Expect((&controller.ClusterStackReleaseReconciler{
5454
Client: testEnv.Manager.GetClient(),
5555
RESTConfig: testEnv.Manager.GetConfig(),
5656
KubeClientFactory: kube.NewFactory(),
57-
GitHubClientFactory: githubclient.NewFactory(),
57+
AssetsClientFactory: githubclient.NewFactory(),
5858
ReleaseDirectory: "/tmp/downloads",
5959
}).SetupWithManager(ctx, testEnv.Manager, controllerruntimecontroller.Options{})).To(Succeed())
6060

‎internal/test/integration/github/integration_test.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,22 @@ var _ = Describe("ClusterStackReconciler", func() {
6969
})
7070

7171
AfterEach(func() {
72-
Expect(testEnv.Cleanup(ctx, testNs, clusterStack)).To(Succeed())
72+
Eventually(func() error {
73+
return testEnv.Cleanup(ctx, testNs, clusterStack)
74+
}, timeout, interval).Should(BeNil())
75+
})
76+
77+
It("checks if the AssetsClientAPIAvailableCondition condition is true", func() {
78+
Eventually(func() bool {
79+
var foundClusterStackRelease csov1alpha1.ClusterStackRelease
80+
if err := testEnv.Get(ctx, clusterStackReleaseKey, &foundClusterStackRelease); err != nil {
81+
testEnv.GetLogger().Error(err, "failed to get clusterStackRelease", "key", clusterStackReleaseKey)
82+
return false
83+
}
84+
85+
testEnv.GetLogger().Info("status condition of cluster stack release", "key", clusterStackReleaseKey, "status condition", foundClusterStackRelease.Status.Conditions)
86+
return utils.IsPresentAndTrue(ctx, testEnv.Client, clusterStackReleaseKey, &foundClusterStackRelease, csov1alpha1.AssetsClientAPIAvailableCondition)
87+
}, timeout, interval).Should(BeTrue())
7388
})
7489

7590
It("creates the cluster stack release object", func() {

0 commit comments

Comments
 (0)