Address reviews

The commit does the following: 1. Fix bundleDeployment names to be camelcase. 2. Remove rukpakctl, upload source and relevant upload mgr code. 3. Remove the requirement for requeing. 4. Remove the use of a separate "Processor" interface for handling unpacked contents before installing. 5. Fix DOckerfile and Goreleaser to not handle rukpakctl binary. Signed-off-by: Varsha Prasad Narsing <[email protected]>
operator-framework · Jan 16, 2024 · 2320883 · 2320883
1 parent 6e22aac
commit 2320883
Show file tree

Hide file tree

Showing 49 changed files with 260 additions and 2,138 deletions.
diff --git a/.goreleaser.yml b/.goreleaser.yml
@@ -63,17 +63,6 @@ builds:
     - arm64
     - ppc64le
     - s390x
-  - id: rukpakctl
-    main: ./cmd/rukpakctl
-    binary: rukpakctl
-    tags: "{{.Env.GO_BUILD_TAGS}}"
-    goos:
-      - linux
-    goarch:
-      - amd64
-      - arm64
-      - ppc64le
-      - s390x
 dockers:
 - image_templates:
   - "{{ .Env.IMAGE_REPO }}:{{ .Env.IMAGE_TAG }}-amd64"

diff --git a/Dockerfile b/Dockerfile
@@ -7,6 +7,5 @@ COPY core core
 COPY unpack unpack
 COPY webhooks webhooks
 COPY crdvalidator crdvalidator
-COPY rukpakctl rukpakctl
 
 EXPOSE 8080
diff --git a/Makefile b/Makefile
@@ -80,7 +80,6 @@ generate: $(CONTROLLER_GEN) ## Generate code and manifests
 		paths=./internal/controllers/bundledeployment/... \
 		paths=./internal/provisioner/plain/... \
 		paths=./internal/provisioner/registry/... \
-		paths=./internal/uploadmgr/... \
 			output:stdout > ./manifests/base/core/resources/cluster_role.yaml
 	$(Q)$(CONTROLLER_GEN) rbac:roleName=webhooks-admin paths=./internal/webhook/... output:stdout > ./manifests/base/apis/webhooks/resources/cluster_role.yaml
 	$(Q)$(CONTROLLER_GEN) rbac:roleName=helm-provisioner-admin \
@@ -111,7 +110,7 @@ test-e2e: $(GINKGO) ## Run the e2e tests
 	$(GINKGO) --tags $(GO_BUILD_TAGS) $(E2E_FLAGS) --trace $(FOCUS) test/e2e
 
 e2e: KIND_CLUSTER_NAME=rukpak-e2e
-e2e: rukpakctl run image-registry local-git kind-load-bundles registry-load-bundles test-e2e kind-cluster-cleanup ## Run e2e tests against an ephemeral kind cluster
+e2e: run image-registry local-git kind-load-bundles registry-load-bundles test-e2e kind-cluster-cleanup ## Run e2e tests against an ephemeral kind cluster
 
 kind-cluster: $(KIND) kind-cluster-cleanup ## Standup a kind cluster
 	$(KIND) create cluster --name ${KIND_CLUSTER_NAME} ${KIND_CLUSTER_CONFIG}
@@ -169,7 +168,7 @@ uninstall: ## Remove all rukpak resources from the cluster
 
 ##@ build/load:
 
-BINARIES=core helm unpack webhooks crdvalidator rukpakctl
+BINARIES=core helm unpack webhooks crdvalidator
 LINUX_BINARIES=$(join $(addprefix linux/,$(BINARIES)), )
 
 .PHONY: build $(BINARIES) $(LINUX_BINARIES) build-container kind-load kind-load-bundles kind-cluster registry-load-bundles

diff --git a/api/v1alpha2/bundle_types.go b/api/v1alpha2/bundle_types.go
@@ -20,11 +20,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
-var (
-	BundleGVK  = SchemeBuilder.GroupVersion.WithKind("Bundle")
-	BundleKind = BundleGVK.Kind
-)
-
 type SourceType string
 
 const (
@@ -58,11 +53,6 @@ type BundleSource struct {
 	// ConfigMaps is a list of config map references and their relative
 	// directory paths that represent a bundle filesystem.
 	ConfigMaps []ConfigMapSource `json:"configMaps,omitempty"`
-	// Upload is a source that enables this Bundle's content to be uploaded
-	// via Rukpak's bundle upload service. This source type is primarily useful
-	// with bundle development workflows because it enables bundle developers
-	// to inject a local bundle directly into the cluster.
-	Upload *UploadSource `json:"upload,omitempty"`
 	//  HTTP is the remote location that backs the content of this Bundle.
 	HTTP *HTTPSource `json:"http,omitempty"`
 }
@@ -131,6 +121,4 @@ type Authorization struct {
 	InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"`
 }
 
-type UploadSource struct{}
-
 type ProvisionerID string
diff --git a/api/v1alpha2/bundledeployment_types.go b/api/v1alpha2/bundledeployment_types.go
@@ -30,8 +30,6 @@ const (
 	TypeHasValidBundle = "HasValidBundle"
 	TypeHealthy        = "Healthy"
 	TypeInstalled      = "Installed"
-	// TypeUploadStatus indicates the status of the bundle content upload by the uploadmgr.
-	TypeUploadStatus = "UploadStatus"
 
 	ReasonBundleLoadFailed          = "BundleLoadFailed"
 	ReasonCreateDynamicWatchFailed  = "CreateDynamicWatchFailed"
@@ -47,8 +45,6 @@ const (
 	ReasonReconcileFailed           = "ReconcileFailed"
 	ReasonUnhealthy                 = "Unhealthy"
 	ReasonUpgradeFailed             = "UpgradeFailed"
-	ReasonUploadSuccessful          = "UploadSuccessful"
-	ReasonUploadFailed              = "UploadFailed"
 )
 
 // BundleDeploymentSpec defines the desired state of BundleDeployment

diff --git a/api/v1alpha2/zz_generated.deepcopy.go b/api/v1alpha2/zz_generated.deepcopy.go
diff --git a/cmd/core/main.go b/cmd/core/main.go
@@ -50,7 +50,6 @@ import (
 	"github.com/operator-framework/rukpak/internal/provisioner/registry"
 	"github.com/operator-framework/rukpak/internal/source"
 	"github.com/operator-framework/rukpak/internal/storage"
-	"github.com/operator-framework/rukpak/internal/uploadmgr"
 	"github.com/operator-framework/rukpak/internal/util"
 	"github.com/operator-framework/rukpak/internal/version"
 	"github.com/operator-framework/rukpak/pkg/features"
@@ -78,10 +77,8 @@ func main() {
 		probeAddr                   string
 		systemNamespace             string
 		unpackImage                 string
-		baseUploadManagerURL        string
 		rukpakVersion               bool
 		provisionerStorageDirectory string
-		uploadStorageDirectory      string
 		uploadStorageSyncInterval   time.Duration
 	)
 	flag.StringVar(&httpBindAddr, "http-bind-address", ":8080", "The address the http server binds to.")
@@ -90,13 +87,11 @@ func main() {
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
 	flag.StringVar(&systemNamespace, "system-namespace", "", "Configures the namespace that gets used to deploy system resources.")
 	flag.StringVar(&unpackImage, "unpack-image", util.DefaultUnpackImage, "Configures the container image that gets used to unpack Bundle contents.")
-	flag.StringVar(&baseUploadManagerURL, "base-upload-manager-url", "", "The base URL from which to fetch uploaded bundles.")
 	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
 		"Enable leader election for controller manager. "+
 			"Enabling this will ensure there is only one active controller manager.")
 	flag.BoolVar(&rukpakVersion, "version", false, "Displays rukpak version information")
 	flag.StringVar(&provisionerStorageDirectory, "provisioner-storage-dir", storage.DefaultBundleCacheDir, "The directory that is used to store bundle contents.")
-	flag.StringVar(&uploadStorageDirectory, "upload-storage-dir", uploadmgr.DefaultBundleCacheDir, "The directory that is used to store bundle uploads.")
 	flag.DurationVar(&uploadStorageSyncInterval, "upload-storage-sync-interval", time.Minute, "Interval on which to garbage collect unused uploaded bundles")
 	opts := zap.Options{
 		Development: true,
@@ -115,7 +110,7 @@ func main() {
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
 	setupLog.Info("starting up the core controllers and servers", "git commit", version.String(), "unpacker image", unpackImage)
 
-	dependentRequirement, err := labels.NewRequirement(util.CoreOwnerKindKey, selection.In, []string{rukpakv1alpha2.BundleKind, rukpakv1alpha2.BundleDeploymentKind})
+	dependentRequirement, err := labels.NewRequirement(util.CoreOwnerKindKey, selection.In, []string{rukpakv1alpha2.BundleDeploymentKind})
 	if err != nil {
 		setupLog.Error(err, "unable to create dependent label selector for cache")
 		os.Exit(1)
@@ -195,14 +190,6 @@ func main() {
 		setupLog.Error(err, "unable to add bundles http handler to manager")
 		os.Exit(1)
 	}
-	if err := mgr.AddMetricsExtraHandler("/uploads/", httpLogger(uploadmgr.NewUploadHandler(mgr.GetClient(), uploadStorageDirectory))); err != nil {
-		setupLog.Error(err, "unable to add uploads http handler to manager")
-		os.Exit(1)
-	}
-	if err := mgr.Add(uploadmgr.NewBundleGC(mgr.GetCache(), uploadStorageDirectory, uploadStorageSyncInterval)); err != nil {
-		setupLog.Error(err, "unable to add bundle garbage collector to manager")
-		os.Exit(1)
-	}
 
 	// This finalizer logic MUST be co-located with this main
 	// controller logic because it deals with cleaning up bundle data
@@ -223,7 +210,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	unpacker, err := source.NewDefaultUnpacker(systemNsCluster, systemNamespace, unpackImage, baseUploadManagerURL, rootCAs)
+	unpacker, err := source.NewDefaultUnpacker(systemNsCluster, systemNamespace, unpackImage, rootCAs)
 	if err != nil {
 		setupLog.Error(err, "unable to setup bundle unpacker")
 		os.Exit(1)
@@ -236,13 +223,12 @@ func main() {
 		bundledeployment.WithActionClientGetter(acg),
 		bundledeployment.WithFinalizers(bundleFinalizers),
 		bundledeployment.WithStorage(bundleStorage),
+		bundledeployment.WithUnpacker(unpacker),
 	}
 
 	if err := bundledeployment.SetupWithManager(mgr, systemNsCluster.GetCache(), systemNamespace, append(
 		commonBDProvisionerOptions,
-		bundledeployment.WithUnpacker(unpacker),
 		bundledeployment.WithProvisionerID(plain.ProvisionerID),
-		bundledeployment.WithBundleDeplymentProcessor(bundledeployment.ProcessorFunc(plain.ProcessBundleDeployment)),
 		bundledeployment.WithHandler(bundledeployment.HandlerFunc(plain.HandleBundleDeployment)),
 	)...); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", rukpakv1alpha2.BundleDeploymentKind, "provisionerID", plain.ProvisionerID)
@@ -251,12 +237,10 @@ func main() {
 
 	if err := bundledeployment.SetupWithManager(mgr, systemNsCluster.GetCache(), systemNamespace, append(
 		commonBDProvisionerOptions,
-		bundledeployment.WithUnpacker(unpacker),
 		bundledeployment.WithProvisionerID(registry.ProvisionerID),
-		bundledeployment.WithBundleDeplymentProcessor(bundledeployment.ProcessorFunc(registry.ProcessBundleDeployment)),
-		bundledeployment.WithHandler(bundledeployment.HandlerFunc(plain.HandleBundleDeployment)),
+		bundledeployment.WithHandler(bundledeployment.HandlerFunc(registry.HandleBundleDeployment)),
 	)...); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", rukpakv1alpha2.BundleDeploymentKind, "provisionerID", plain.ProvisionerID)
+		setupLog.Error(err, "unable to create controller", "controller", rukpakv1alpha2.BundleDeploymentKind, "provisionerID", registry.ProvisionerID)
 		os.Exit(1)
 	}
 	//+kubebuilder:scaffold:builder

diff --git a/cmd/helm/main.go b/cmd/helm/main.go
@@ -68,7 +68,6 @@ func main() {
 		probeAddr            string
 		systemNamespace      string
 		unpackImage          string
-		baseUploadManagerURL string
 		rukpakVersion        bool
 		storageDirectory     string
 	)
@@ -77,7 +76,6 @@ func main() {
 	flag.StringVar(&bundleCAFile, "bundle-ca-file", "", "The file containing the certificate authority for connecting to bundle content servers.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
 	flag.StringVar(&unpackImage, "unpack-image", util.DefaultUnpackImage, "Configures the container image that gets used to unpack Bundle contents.")
-	flag.StringVar(&baseUploadManagerURL, "base-upload-manager-url", "", "The base URL from which to fetch uploaded bundles.")
 	flag.StringVar(&systemNamespace, "system-namespace", "", "Configures the namespace that gets used to deploy system resources.")
 	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
 		"Enable leader election for controller manager. "+
@@ -197,7 +195,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	unpacker, err := source.NewDefaultUnpacker(systemNsCluster, systemNamespace, unpackImage, baseUploadManagerURL, rootCAs)
+	unpacker, err := source.NewDefaultUnpacker(systemNsCluster, systemNamespace, unpackImage, rootCAs)
 	if err != nil {
 		setupLog.Error(err, "unable to setup bundle unpacker")
 		os.Exit(1)
@@ -210,13 +208,12 @@ func main() {
 		bundledeployment.WithFinalizers(bundleFinalizers),
 		bundledeployment.WithActionClientGetter(acg),
 		bundledeployment.WithStorage(bundleStorage),
+		bundledeployment.WithUnpacker(unpacker),
 	}
 
 	if err := bundledeployment.SetupWithManager(mgr, systemNsCluster.GetCache(), systemNamespace, append(
 		commonBDProvisionerOptions,
 		bundledeployment.WithProvisionerID(helm.ProvisionerID),
-		bundledeployment.WithUnpacker(unpacker),
-		bundledeployment.WithBundleDeplymentProcessor(bundledeployment.ProcessorFunc(helm.ProcessBundleDeployment)),
 		bundledeployment.WithHandler(bundledeployment.HandlerFunc(helm.HandleBundleDeployment)),
 	)...); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", rukpakv1alpha2.BundleDeploymentKind, "provisionerID", helm.ProvisionerID)

diff --git a/cmd/rukpakctl/LICENSE b/cmd/rukpakctl/LICENSE
diff --git a/cmd/rukpakctl/cmd/alpha.go b/cmd/rukpakctl/cmd/alpha.go