Skip to content
This repository has been archived by the owner on Aug 12, 2024. It is now read-only.

Commit

Permalink
Address reviews
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
varshaprasad96 committed Jan 15, 2024
1 parent 3fae782 commit a17a695
Show file tree
Hide file tree
Showing 49 changed files with 261 additions and 2,130 deletions.
11 changes: 0 additions & 11 deletions .goreleaser.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,5 @@ COPY core core
COPY unpack unpack
COPY webhooks webhooks
COPY crdvalidator crdvalidator
COPY rukpakctl rukpakctl

EXPOSE 8080
5 changes: 2 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down
12 changes: 0 additions & 12 deletions api/v1alpha2/bundle_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ import (
corev1 "k8s.io/api/core/v1"
)

var (
BundleGVK = SchemeBuilder.GroupVersion.WithKind("Bundle")
BundleKind = BundleGVK.Kind
)

type SourceType string

const (
Expand Down Expand Up @@ -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"`
}
Expand Down Expand Up @@ -131,6 +121,4 @@ type Authorization struct {
InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"`
}

type UploadSource struct{}

type ProvisionerID string
4 changes: 0 additions & 4 deletions api/v1alpha2/bundledeployment_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -47,8 +45,6 @@ const (
ReasonReconcileFailed = "ReconcileFailed"
ReasonUnhealthy = "Unhealthy"
ReasonUpgradeFailed = "UpgradeFailed"
ReasonUploadSuccessful = "UploadSuccessful"
ReasonUploadFailed = "UploadFailed"
)

// BundleDeploymentSpec defines the desired state of BundleDeployment
Expand Down
20 changes: 0 additions & 20 deletions api/v1alpha2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 5 additions & 21 deletions cmd/core/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.")
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand Down
7 changes: 2 additions & 5 deletions cmd/helm/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ func main() {
probeAddr string
systemNamespace string
unpackImage string
baseUploadManagerURL string
rukpakVersion bool
storageDirectory string
)
Expand All @@ -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. "+
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
Empty file removed cmd/rukpakctl/LICENSE
Empty file.
39 changes: 0 additions & 39 deletions cmd/rukpakctl/cmd/alpha.go

This file was deleted.

Loading

0 comments on commit a17a695

Please sign in to comment.