Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"github.com/openshift/platform-operators/controllers"
"github.com/openshift/platform-operators/internal/clusteroperator"
"github.com/openshift/platform-operators/internal/sourcer"
"github.com/openshift/platform-operators/internal/util"
//+kubebuilder:scaffold:imports
)

Expand All @@ -56,14 +57,18 @@ func init() {
}

func main() {
var metricsAddr string
var enableLeaderElection bool
var probeAddr string
var (
metricsAddr string
enableLeaderElection bool
probeAddr string
systemNamespace string
)
flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
flag.BoolVar(&enableLeaderElection, "leader-elect", false,
"Enable leader election for controller manager. "+
"Enabling this will ensure there is only one active controller manager.")
flag.StringVar(&systemNamespace, "system-namespace", "openshift-platform-operators", "Configures the namespace that gets used to deploy system resources.")
opts := zap.Options{
Development: true,
}
Expand Down Expand Up @@ -96,8 +101,9 @@ func main() {

// Add Aggregated CO controller to manager
if err = (&controllers.AggregatedClusterOperatorReconciler{
Client: mgr.GetClient(),
ReleaseVersion: clusteroperator.GetReleaseVariable(),
Client: mgr.GetClient(),
ReleaseVersion: clusteroperator.GetReleaseVariable(),
SystemNamespace: util.PodNamespace(systemNamespace),
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "AggregatedCO")
os.Exit(1)
Expand Down

This file was deleted.

22 changes: 22 additions & 0 deletions config/clusteroperator/aggregated_clusteroperator.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
apiVersion: config.openshift.io/v1
kind: ClusterOperator
metadata:
name: aggregated
spec: {}
status:
versions:
- name: operator
version: "0.0.1-snapshot"
relatedObjects:
- group: ''
name: openshift-platform-operators
resource: namespaces
- group: platform.openshift.io
name: ""
resource: platformoperators
- group: core.rukpak.io
name: ""
resource: bundles
- group: core.rukpak.io
name: ""
resource: bundledeployments
Comment on lines +17 to +22
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prior to GA, I think we should separate out the rukpak stuff from the PO stuff. At the end of the day, PO will depend on rukpak, not provide it.

But this is fine in the mean time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup +1.

2 changes: 1 addition & 1 deletion config/default/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ bases:
- ../rbac
- ../manager
- ../rukpak
- ../aggregatedclusteroperator
- ../clusteroperator
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
# crd/kustomization.yaml
#- ../webhook
Expand Down
4 changes: 4 additions & 0 deletions config/default/manager_auth_proxy_patch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ spec:
- containerPort: 8443
protocol: TCP
name: https
resources:
requests:
cpu: 1m
memory: 15Mi
Comment on lines +23 to +26
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: these requests are for the kube-rbac-proxy sidecar container. I modeled these after what cluster monitoring have defined for their deployments that utilize kube-rbac-proxy.

securityContext:
allowPrivilegeEscalation: false
capabilities:
Expand Down
6 changes: 1 addition & 5 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ spec:
runAsNonRoot: true
seccompProfile:
type: RuntimeDefault
priorityClassName: "system-cluster-critical"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: just mirroring what's defined for OLM sub-components here.

containers:
- command:
- /manager
Expand Down Expand Up @@ -57,12 +58,7 @@ spec:
port: 8081
initialDelaySeconds: 5
periodSeconds: 10
# TODO(user): Configure the resources accordingly based on the project requirements.
# More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
resources:
limits:
cpu: 500m
memory: 128Mi
Comment on lines -63 to -65
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: we have conformance tests that verify component pods don't specify limits, and don't get assigned the BestEffort quality of service. Removing the limits resulted in the Burstable QoS being assigned to these pods.

requests:
cpu: 10m
memory: 64Mi
Expand Down
5 changes: 5 additions & 0 deletions config/rukpak/apis/webhooks/resources/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ spec:
runAsNonRoot: true
seccompProfile:
type: RuntimeDefault
priorityClassName: "system-cluster-critical"
serviceAccountName: webhooks-admin
containers:
- name: webhooks
Expand All @@ -32,6 +33,10 @@ spec:
- containerPort: 9443
name: webhook-server
protocol: TCP
resources:
requests:
cpu: 10m
memory: 50Mi
volumeMounts:
- mountPath: /tmp/k8s-webhook-server/serving-certs
name: cert
Expand Down
9 changes: 9 additions & 0 deletions config/rukpak/core/resources/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ spec:
runAsNonRoot: true
seccompProfile:
type: RuntimeDefault
priorityClassName: "system-cluster-critical"
containers:
- name: kube-rbac-proxy
securityContext:
Expand All @@ -41,6 +42,10 @@ spec:
- containerPort: 8443
protocol: TCP
name: https
resources:
requests:
cpu: 1m
memory: 15Mi
volumeMounts:
- name: tls
mountPath: /etc/pki/tls
Expand All @@ -65,6 +70,10 @@ spec:
- "--bundle-ca-file=/etc/pki/tls/tls.crt"
ports:
- containerPort: 8080
resources:
requests:
cpu: 15m
memory: 100Mi
Comment on lines +73 to +76
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the core rukpak deployment. I increased the memory request to 100Mi just because we're caching bundle contents in-memory, so we may want to invest in benchmarking this behavior to get a better read on what kind of requests we need to be setting for subsequent phases.

volumeMounts:
- name: bundle-cache
mountPath: /var/cache/bundles
Expand Down
21 changes: 13 additions & 8 deletions controllers/aggregated_clusteroperator_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ import (

type AggregatedClusterOperatorReconciler struct {
client.Client
ReleaseVersion string
ReleaseVersion string
SystemNamespace string
}

const aggregateCOName = "platform-operators-aggregated"
Expand All @@ -49,16 +50,16 @@ const aggregateCOName = "platform-operators-aggregated"
//
// For more details, check Reconcile and its Result here:
// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.10.0/pkg/reconcile
func (a *AggregatedClusterOperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
func (r *AggregatedClusterOperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
log := logr.FromContext(ctx)
log.Info("reconciling request", "req", req.NamespacedName)
defer log.Info("finished reconciling request", "req", req.NamespacedName)

coBuilder := clusteroperator.NewBuilder()
coWriter := clusteroperator.NewWriter(a.Client)
coWriter := clusteroperator.NewWriter(r.Client)

aggregatedCO := &openshiftconfigv1.ClusterOperator{}
if err := a.Get(ctx, req.NamespacedName, aggregatedCO); err != nil {
if err := r.Get(ctx, req.NamespacedName, aggregatedCO); err != nil {
return ctrl.Result{}, client.IgnoreNotFound(err)
}
defer func() {
Expand All @@ -71,10 +72,14 @@ func (a *AggregatedClusterOperatorReconciler) Reconcile(ctx context.Context, req
coBuilder.WithProgressing(openshiftconfigv1.ConditionTrue, "")
coBuilder.WithDegraded(openshiftconfigv1.ConditionFalse)
coBuilder.WithAvailable(openshiftconfigv1.ConditionFalse, "", "")
coBuilder.WithVersion("operator", a.ReleaseVersion)
coBuilder.WithVersion("operator", r.ReleaseVersion)
coBuilder.WithRelatedObject("", "namespaces", "", r.SystemNamespace)
coBuilder.WithRelatedObject("platform.openshift.io", "platformoperators", "", "")
coBuilder.WithRelatedObject("core.rukpak.io", "bundles", "", "")
coBuilder.WithRelatedObject("core.rukpak.io", "bundledeployments", "", "")

poList := &platformv1alpha1.PlatformOperatorList{}
if err := a.List(ctx, poList); err != nil {
if err := r.List(ctx, poList); err != nil {
return ctrl.Result{}, err
}
if len(poList.Items) == 0 {
Expand All @@ -98,11 +103,11 @@ func (a *AggregatedClusterOperatorReconciler) Reconcile(ctx context.Context, req
}

// SetupWithManager sets up the controller with the Manager.
func (a *AggregatedClusterOperatorReconciler) SetupWithManager(mgr ctrl.Manager) error {
func (r *AggregatedClusterOperatorReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&openshiftconfigv1.ClusterOperator{}, builder.WithPredicates(predicate.NewPredicateFuncs(func(object client.Object) bool {
return object.GetName() == aggregateCOName
}))).
Watches(&source.Kind{Type: &platformv1alpha1.PlatformOperator{}}, handler.EnqueueRequestsFromMapFunc(util.RequeueClusterOperator(mgr.GetClient(), aggregateCOName))).
Complete(a)
Complete(r)
}
13 changes: 13 additions & 0 deletions internal/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package util
import (
"context"
"fmt"
"os"

configv1 "github.com/openshift/api/config/v1"
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
Expand All @@ -18,6 +19,18 @@ import (
platformtypes "github.com/openshift/platform-operators/api/v1alpha1"
)

// GetPodNamespace checks whether the controller is running in a Pod vs.
// being run locally by inspecting the namespace file that gets mounted
// automatically for Pods at runtime. If that file doesn't exist, then
// return the value of the defaultNamespace parameter.
func PodNamespace(defaultNamespace string) string {
namespace, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace")
if err != nil {
return defaultNamespace
}
return string(namespace)
}

func RequeuePlatformOperators(cl client.Client) handler.MapFunc {
return func(object client.Object) []reconcile.Request {
poList := &platformv1alpha1.PlatformOperatorList{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ spec:
- containerPort: 8443
name: https
protocol: TCP
resources:
requests:
cpu: 1m
memory: 15Mi
securityContext:
allowPrivilegeEscalation: false
capabilities:
Expand All @@ -68,6 +72,10 @@ spec:
name: manager
ports:
- containerPort: 8080
resources:
requests:
cpu: 15m
memory: 100Mi
securityContext:
allowPrivilegeEscalation: false
capabilities:
Expand All @@ -80,6 +88,7 @@ spec:
name: upload-cache
- mountPath: /etc/pki/tls
name: tls
priorityClassName: system-cluster-critical
securityContext:
runAsNonRoot: true
seccompProfile:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ spec:
- containerPort: 9443
name: webhook-server
protocol: TCP
resources:
requests:
cpu: 10m
memory: 50Mi
securityContext:
allowPrivilegeEscalation: false
capabilities:
Expand All @@ -55,6 +59,7 @@ spec:
- mountPath: /tmp/k8s-webhook-server/serving-certs
name: cert
readOnly: true
priorityClassName: system-cluster-critical
securityContext:
runAsNonRoot: true
seccompProfile:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ spec:
- containerPort: 8443
name: https
protocol: TCP
resources:
requests:
cpu: 1m
memory: 15Mi
securityContext:
allowPrivilegeEscalation: false
capabilities:
Expand Down Expand Up @@ -68,9 +72,6 @@ spec:
initialDelaySeconds: 5
periodSeconds: 10
resources:
limits:
cpu: 500m
memory: 128Mi
requests:
cpu: 10m
memory: 64Mi
Expand All @@ -79,6 +80,7 @@ spec:
capabilities:
drop:
- ALL
priorityClassName: system-cluster-critical
securityContext:
runAsNonRoot: true
seccompProfile:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,19 @@ metadata:
namespace: openshift-platform-operators
spec: {}
status:
relatedObjects:
- group: ""
name: openshift-platform-operators
resource: namespaces
- group: platform.openshift.io
name: ""
resource: platformoperators
- group: core.rukpak.io
name: ""
resource: bundles
- group: core.rukpak.io
name: ""
resource: bundledeployments
versions:
- name: operator
version: 0.0.1-snapshot
11 changes: 10 additions & 1 deletion test/e2e/aggregated_clusteroperator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var _ = Describe("aggregated clusteroperator controller", func() {
WithTransform(func(c *configv1.ClusterOperatorStatusCondition) string { return c.Message }, ContainSubstring("No POs are present in the cluster")),
))
})
It("should consistently contain a populated status.versions", func() {
It("should consistently contain a populated status.versions array", func() {
Consistently(func() (bool, error) {
co := &configv1.ClusterOperator{}
if err := c.Get(ctx, types.NamespacedName{Name: aggregateCOName}, co); err != nil {
Expand All @@ -57,6 +57,15 @@ var _ = Describe("aggregated clusteroperator controller", func() {
return version.Name != "" && version.Version != "", nil
}).Should(BeTrue())
})
It("should consistently contain a populated status.relatedObjects array", func() {
Consistently(func() (bool, error) {
co := &configv1.ClusterOperator{}
if err := c.Get(ctx, types.NamespacedName{Name: aggregateCOName}, co); err != nil {
return false, err
}
return len(co.Status.RelatedObjects) == 4, nil
}).Should(BeTrue())
})
})

When("installing a series of POs successfully", func() {
Expand Down