Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consoleplugin reconciler #884

Merged
merged 12 commits into from
Oct 8, 2024
10 changes: 10 additions & 0 deletions .github/workflows/build-images-base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ on:
description: WASM Shim version
default: latest
type: string
consolePluginImageURL:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@didierofrivia @maleck13 trying here complete URL instead of some "version". What are your thoughts about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry guys, accepted by (answer) omission.

We can always change that later

description: ConsolePlugin image URL
default: "quay.io/kuadrant/console-plugin:latest"
type: string
channels:
description: Bundle and catalog channels, comma separated
default: preview
Expand Down Expand Up @@ -65,6 +69,10 @@ on:
description: WASM Shim version
default: latest
type: string
consolePluginImageURL:
description: ConsolePlugin image URL
default: "quay.io/kuadrant/console-plugin:latest"
type: string
channels:
description: Bundle and catalog channels, comma separated
default: preview
Expand Down Expand Up @@ -147,6 +155,7 @@ jobs:
LIMITADOR_OPERATOR_VERSION=${{ inputs.limitadorOperatorVersion }} \
DNS_OPERATOR_VERSION=${{ inputs.dnsOperatorVersion }} \
WASM_SHIM_VERSION=${{ inputs.wasmShimVersion }} \
RELATED_IMAGE_CONSOLEPLUGIN=${{ inputs.consolePluginImageURL }} \
DEFAULT_CHANNEL=${{ inputs.defaultChannel }} \
CHANNELS=${{ inputs.channels }}
- name: Set up Docker Buildx
Expand Down Expand Up @@ -195,6 +204,7 @@ jobs:
LIMITADOR_OPERATOR_VERSION=${{ inputs.limitadorOperatorVersion }} \
DNS_OPERATOR_VERSION=${{ inputs.dnsOperatorVersion }} \
WASM_SHIM_VERSION=${{ inputs.wasmShimVersion }} \
RELATED_IMAGE_CONSOLEPLUGIN=${{ inputs.consolePluginImageURL }} \
DEFAULT_CHANNEL=${{ inputs.defaultChannel }}
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3
Expand Down
7 changes: 6 additions & 1 deletion .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ on:
description: WASM Shim version
default: latest
type: string
consolePluginImageURL:
description: ConsolePlugin image URL
default: "quay.io/kuadrant/console-plugin:latest"
type: string
prerelease:
description: Is the release a pre-release?
required: false
Expand Down Expand Up @@ -60,6 +64,7 @@ jobs:
LIMITADOR_OPERATOR_VERSION=${{ inputs.limitadorOperatorVersion }} \
DNS_OPERATOR_VERSION=${{ inputs.dnsOperatorVersion }} \
WASM_SHIM_VERSION=${{ inputs.wasmShimVersion }} \
RELATED_IMAGE_CONSOLEPLUGIN=${{ inputs.consolePluginImageURL }} \
make prepare-release
- name: Commit and push
run: |
Expand All @@ -72,7 +77,7 @@ jobs:
with:
name: v${{ inputs.kuadrantOperatorVersion }}
tag_name: v${{ inputs.kuadrantOperatorVersion }}
body: "**This release enables installations of Authorino Operator v${{ inputs.authorinoOperatorVersion }}, Limitador Operator v${{ inputs.limitadorOperatorVersion }}, DNS Operator v${{ inputs.dnsOperatorVersion }} and WASM Shim v${{ inputs.wasmShimVersion }}**"
body: "**This release enables installations of Authorino Operator v${{ inputs.authorinoOperatorVersion }}, Limitador Operator v${{ inputs.limitadorOperatorVersion }}, DNS Operator v${{ inputs.dnsOperatorVersion }}, WASM Shim v${{ inputs.wasmShimVersion }} and ConsolePlugin ${{ inputs.consolePluginImageURL }}**"
generate_release_notes: true
target_commitish: release-v${{ github.event.inputs.kuadrantOperatorVersion }}
prerelease: ${{ github.event.inputs.prerelease }}
14 changes: 11 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ build: generate fmt vet ## Build manager binary.

run: export LOG_LEVEL = debug
run: export LOG_MODE = development
run: export OPERATOR_NAMESPACE = kuadrant-system
run: export OPERATOR_NAMESPACE = $(shell kubectl config view --minify -o jsonpath='{..namespace}')
Copy link
Contributor

Choose a reason for hiding this comment

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

This gives an empty value for me for a kind and crc cluster, so it will panic for most people using make run here unless it's just me 🤔

panic("namespace must be specified and can not be a blank string")

{
    "kind": "Config",
    "apiVersion": "v1",
    "preferences": {},
    "clusters": [
        {
            "name": "api-crc-testing:6443",
            "cluster": {
                "server": "https://api.crc.testing:6443",
                "certificate-authority-data": "DATA+OMITTED"
            }
        }
    ],
    "users": [
        {
            "name": "developer/api-crc-testing:6443",
            "user": {
                "token": "REDACTED"
            }
        }
    ],
    "contexts": [
        {
            "name": "/api-crc-testing:6443/developer",
            "context": {
                "cluster": "api-crc-testing:6443",
                "user": "developer/api-crc-testing:6443"
            }
        }
    ],
    "current-context": "/api-crc-testing:6443/developer"
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The command line kubectl config view --minify -o jsonpath='{..namespace}' is pretty common way to parse the current context's namespace. I do not think there is anything wrong with it.

Your kubeconfig does not have namespace set on the current context. This is a portion of my kubeconfig

contexts:
- context:                                                                 
    cluster: mycluster:443                   
    namespace: kuadrant-system                                             
    user: eguzki/mycluster:443               
  name: kuadrant-system/mycluster:443/eguzki 
current-context: kuadrant-system/mycluster:443/eguzki 

Maybe oc project kuadrant-system will help??

I will try to enhance makefile command to handle error when the namespace is not available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, yeah adding oc project kuadrant-system will add the namespace to the project, but without it there no namespace in the context

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I don't know about this change tbh. I'm getting the following whenever I run make run on a fresh kind cluster made using make local-env-setup, so this will affect the make run command once merged to main.

🚨 namespace could not be parsed from kubeconfig 💥

This does get set on a CRC cluster as I was interfacting with the cluster by following the verification steps. So I assume this gets set typically when interacting with an openshift type cluster unless I'm missing something 🤔

TBH the env var has a default now also so we probably don't need this set in the makefile also unless people want to explicitly change it

operatorNamespace = env.GetString("OPERATOR_NAMESPACE", "kuadrant-system")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I wanted to read the namespace from the kubeconfig context, but turns out that in local env (kind), there is no namespace for the current context.

Reverting to previous state with the addition of adding OPERATOR_NAMESPACE makefile variable to override default value (kuadrant-system).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I think that for make run, the OPERATOR_NAMESPACE should be:

  • When OPERATOR_NAMESPACE makefile variable is set, it takes that value. Else
  • namespace from kubeconfig current context. If empty, then
  • default from KUADRANT_NAMESPACE makefile variable, i.e. kuadrant-system.

But we can leave this for other PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @Boomatang ^^

run: GIT_SHA=$(shell git rev-parse HEAD || echo "unknown")
run: DIRTY=$(shell $(PROJECT_PATH)/utils/check-git-dirty.sh || echo "unknown")
run: generate fmt vet ## Run a controller from your host.
Expand Down Expand Up @@ -383,12 +383,18 @@ rm -rf $$TMP_DIR ;\
}
endef

RELATED_IMAGE_CONSOLEPLUGIN ?= quay.io/kuadrant/console-plugin:latest

.PHONY: bundle
bundle: $(OPM) $(YQ) manifests dependencies-manifests kustomize operator-sdk ## Generate bundle manifests and metadata, then validate generated files.
$(OPERATOR_SDK) generate kustomize manifests -q
# Set desired operator image and related wasm shim image
# Set desired Wasm-shim image
V="$(RELATED_IMAGE_WASMSHIM)" \
$(YQ) eval '(select(.kind == "Deployment").spec.template.spec.containers[].env[] | select(.name == "RELATED_IMAGE_WASMSHIM").value) = strenv(V)' -i config/manager/manager.yaml
# Set desired ConsolePlugin image
V="$(RELATED_IMAGE_CONSOLEPLUGIN)" \
$(YQ) eval '(select(.kind == "Deployment").spec.template.spec.containers[].env[] | select(.name == "RELATED_IMAGE_CONSOLEPLUGIN").value) = strenv(V)' -i config/manager/manager.yaml
# Set desired operator image
cd config/manager && $(KUSTOMIZE) edit set image controller=$(IMG)
# Update CSV
$(call update-csv-config,kuadrant-operator.v$(BUNDLE_VERSION),config/manifests/bases/kuadrant-operator.clusterserviceversion.yaml,.metadata.name)
Expand Down Expand Up @@ -443,11 +449,13 @@ prepare-release: ## Prepare the manifests for OLM and Helm Chart for a release.
LIMITADOR_OPERATOR_VERSION=$(LIMITADOR_OPERATOR_VERSION) \
DNS_OPERATOR_VERSION=$(DNS_OPERATOR_VERSION) \
WASM_SHIM_VERSION=$(WASM_SHIM_VERSION) \
RELATED_IMAGE_CONSOLEPLUGIN=$(RELATED_IMAGE_CONSOLEPLUGIN) \
$(MAKE) helm-build VERSION=$(VERSION) \
AUTHORINO_OPERATOR_VERSION=$(AUTHORINO_OPERATOR_VERSION) \
LIMITADOR_OPERATOR_VERSION=$(LIMITADOR_OPERATOR_VERSION) \
DNS_OPERATOR_VERSION=$(DNS_OPERATOR_VERSION) \
WASM_SHIM_VERSION=$(WASM_SHIM_VERSION)
WASM_SHIM_VERSION=$(WASM_SHIM_VERSION) \
RELATED_IMAGE_CONSOLEPLUGIN=$(RELATED_IMAGE_CONSOLEPLUGIN)
sed -i -e 's/Version = ".*"/Version = "$(VERSION)"/' $(PROJECT_PATH)/version/version.go

##@ Code Style
Expand Down
16 changes: 16 additions & 0 deletions bundle/manifests/kuadrant-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,18 @@ spec:
- get
- list
- watch
- apiGroups:
- console.openshift.io
resources:
- consoleplugins
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- coordination.k8s.io
resources:
Expand Down Expand Up @@ -648,6 +660,8 @@ spec:
env:
- name: RELATED_IMAGE_WASMSHIM
value: oci://quay.io/kuadrant/wasm-shim:latest
- name: RELATED_IMAGE_CONSOLEPLUGIN
value: quay.io/kuadrant/console-plugin:latest
- name: OPERATOR_NAMESPACE
valueFrom:
fieldRef:
Expand Down Expand Up @@ -756,4 +770,6 @@ spec:
relatedImages:
- image: oci://quay.io/kuadrant/wasm-shim:latest
name: wasmshim
- image: quay.io/kuadrant/console-plugin:latest
name: consoleplugin
version: 0.0.0
14 changes: 14 additions & 0 deletions charts/kuadrant-operator/templates/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15750,6 +15750,18 @@ rules:
- get
- list
- watch
- apiGroups:
- console.openshift.io
resources:
- consoleplugins
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- coordination.k8s.io
resources:
Expand Down Expand Up @@ -16212,6 +16224,8 @@ spec:
env:
- name: RELATED_IMAGE_WASMSHIM
value: oci://quay.io/kuadrant/wasm-shim:latest
- name: RELATED_IMAGE_CONSOLEPLUGIN
value: quay.io/kuadrant/console-plugin:latest
- name: OPERATOR_NAMESPACE
valueFrom:
fieldRef:
Expand Down
2 changes: 2 additions & 0 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ spec:
env:
- name: RELATED_IMAGE_WASMSHIM
value: "oci://quay.io/kuadrant/wasm-shim:latest"
- name: RELATED_IMAGE_CONSOLEPLUGIN
value: "quay.io/kuadrant/console-plugin:latest"
- name: OPERATOR_NAMESPACE
valueFrom:
fieldRef:
Expand Down
12 changes: 12 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,18 @@ rules:
- get
- list
- watch
- apiGroups:
- console.openshift.io
resources:
- consoleplugins
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- coordination.k8s.io
resources:
Expand Down
128 changes: 128 additions & 0 deletions controllers/consoleplugin_reconciler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
package controllers

import (
"context"
"sync"

"github.com/go-logr/logr"
consolev1 "github.com/openshift/api/console/v1"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/utils/env"
"k8s.io/utils/ptr"
ctrlruntime "sigs.k8s.io/controller-runtime"

"github.com/kuadrant/policy-machinery/controller"
"github.com/kuadrant/policy-machinery/machinery"

"github.com/kuadrant/kuadrant-operator/pkg/library/reconcilers"
"github.com/kuadrant/kuadrant-operator/pkg/library/utils"
"github.com/kuadrant/kuadrant-operator/pkg/log"
"github.com/kuadrant/kuadrant-operator/pkg/openshift"
"github.com/kuadrant/kuadrant-operator/pkg/openshift/consoleplugin"
)

//+kubebuilder:rbac:groups=console.openshift.io,resources=consoleplugins,verbs=get;list;watch;create;update;patch;delete

var (
ConsolePluginImageURL = env.GetString("RELATED_IMAGE_CONSOLEPLUGIN", "quay.io/kuadrant/console-plugin:latest")
)

type ConsolePluginReconciler struct {
*reconcilers.BaseReconciler

namespace string
}

func NewConsolePluginReconciler(mgr ctrlruntime.Manager, namespace string) *ConsolePluginReconciler {
return &ConsolePluginReconciler{
BaseReconciler: reconcilers.NewBaseReconciler(
mgr.GetClient(), mgr.GetScheme(), mgr.GetAPIReader(),
log.Log.WithName("consoleplugin"),
mgr.GetEventRecorderFor("ConsolePlugin"),
),
namespace: namespace,

Check warning on line 44 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L37-L44

Added lines #L37 - L44 were not covered by tests
}
}

func (r *ConsolePluginReconciler) Subscription() *controller.Subscription {
return &controller.Subscription{
ReconcileFunc: r.Run,
Events: []controller.ResourceEventMatcher{
{Kind: ptr.To(openshift.ConsolePluginGVK.GroupKind())},

Check warning on line 52 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L48-L52

Added lines #L48 - L52 were not covered by tests
{
Kind: ptr.To(ConfigMapGroupKind),
ObjectNamespace: r.namespace,
ObjectName: TopologyConfigMapName,
EventType: ptr.To(controller.CreateEvent),
},

Check warning on line 58 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L54-L58

Added lines #L54 - L58 were not covered by tests
{
Kind: ptr.To(ConfigMapGroupKind),
ObjectNamespace: r.namespace,
ObjectName: TopologyConfigMapName,
EventType: ptr.To(controller.DeleteEvent),
},
},

Check warning on line 65 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L60-L65

Added lines #L60 - L65 were not covered by tests
}
}

func (r *ConsolePluginReconciler) Run(eventCtx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, _ *sync.Map) error {
logger := r.Logger()
logger.V(1).Info("task started")
ctx := logr.NewContext(eventCtx, logger)

Check warning on line 72 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L69-L72

Added lines #L69 - L72 were not covered by tests

existingTopologyConfigMaps := topology.Objects().Items(func(object machinery.Object) bool {
return object.GetName() == TopologyConfigMapName && object.GetNamespace() == r.namespace && object.GroupVersionKind().Kind == ConfigMapGroupKind.Kind
})

Check warning on line 76 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L74-L76

Added lines #L74 - L76 were not covered by tests

topologyExists := len(existingTopologyConfigMaps) > 0

Check warning on line 78 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L78

Added line #L78 was not covered by tests

// Service
service := consoleplugin.Service(r.namespace)
if !topologyExists {
utils.TagObjectToDelete(service)

Check warning on line 83 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L81-L83

Added lines #L81 - L83 were not covered by tests
}
err := r.ReconcileResource(ctx, &corev1.Service{}, service, reconcilers.CreateOnlyMutator)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess if we want to align with the conventions agreed, we do not want to use this r.ReconcileResource function, since it performs a Get from the api server. Instead we want to get this resource from the topology 🤔
Although not sure do we want to block this PR for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ReconcileResource has some functionality we still does not have in the new approach. Do you mind letting this implementation go through? In exchange, I will open issue to address that as tech debt

BTW, I realize that the old issue of endless reconciliation loop for authconfig #354 we fixed with the dry run https://github.com/Kuadrant/kuadrant-operator/blob/main/pkg/library/reconcilers/base_reconciler.go#L156

desired.SetResourceVersion(obj.GetResourceVersion())
	if err := b.Client().Update(ctx, desired, client.DryRunAll); err != nil {
		return err
	}

can now become an issue again with the topology being the source of truth.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I'm fine with leaving it to be addressed later 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created issue #916

if err != nil {
logger.Error(err, "reconciling service")
return err

Check warning on line 88 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L85-L88

Added lines #L85 - L88 were not covered by tests
}

// Deployment
deployment := consoleplugin.Deployment(r.namespace, ConsolePluginImageURL)
deploymentMutators := make([]reconcilers.DeploymentMutateFn, 0)
deploymentMutators = append(deploymentMutators, reconcilers.DeploymentImageMutator)
if !topologyExists {
utils.TagObjectToDelete(deployment)

Check warning on line 96 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L92-L96

Added lines #L92 - L96 were not covered by tests
}
err = r.ReconcileResource(ctx, &appsv1.Deployment{}, deployment, reconcilers.DeploymentMutator(deploymentMutators...))
if err != nil {
logger.Error(err, "reconciling deployment")
return err

Check warning on line 101 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L98-L101

Added lines #L98 - L101 were not covered by tests
}

// Nginx ConfigMap
nginxConfigMap := consoleplugin.NginxConfigMap(r.namespace)
if !topologyExists {
utils.TagObjectToDelete(nginxConfigMap)

Check warning on line 107 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L105-L107

Added lines #L105 - L107 were not covered by tests
}
err = r.ReconcileResource(ctx, &corev1.ConfigMap{}, nginxConfigMap, reconcilers.CreateOnlyMutator)
if err != nil {
logger.Error(err, "reconciling nginx configmap")
return err

Check warning on line 112 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L109-L112

Added lines #L109 - L112 were not covered by tests
}

// ConsolePlugin
consolePlugin := consoleplugin.ConsolePlugin(r.namespace)
if !topologyExists {
utils.TagObjectToDelete(consolePlugin)

Check warning on line 118 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L116-L118

Added lines #L116 - L118 were not covered by tests
}
err = r.ReconcileResource(ctx, &consolev1.ConsolePlugin{}, consolePlugin, reconcilers.CreateOnlyMutator)
if err != nil {
logger.Error(err, "reconciling consoleplugin")
return err

Check warning on line 123 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L120-L123

Added lines #L120 - L123 were not covered by tests
}

logger.V(1).Info("task ended")
return nil

Check warning on line 127 in controllers/consoleplugin_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/consoleplugin_reconciler.go#L126-L127

Added lines #L126 - L127 were not covered by tests
}
Loading
Loading