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
Merged

consoleplugin reconciler #884

merged 12 commits into from
Oct 8, 2024

Conversation

eguzki
Copy link
Contributor

@eguzki eguzki commented Sep 27, 2024

What

Deployment of Kuadrant's Openshift dynamic plugin.

ConsolePlugin v1.0 available on OCP >= 4.12 https://docs.openshift.com/container-platform/4.12/rest_api/console_apis/consoleplugin-console-openshift-io-v1.html

The operator deploys the consoleplugin resources together with the service,deployment and configuration configmap in the operator's namespace

The lifecycle of the consoleplugin resource, together with the related deployment resources, is linked to the lifecycle of the topology configmap. When the topology configmap is created, so is the consoleplugin (and related resources). Same when deleting the topology configmap.

The consoleplugin will be deployed only on clusters where the CRDs are present, namely Openshift.

Fixes Kuadrant/kuadrant-console-plugin#46

TODO:

Verification Steps

Verified on OpenShift v4.15

  • Install Gateway API
make gateway-api-install
  • Install Istio

Follow user guide

Once istio is installed, we can deploy gateways and routes to be shown in the console plugin 🎨

  • Deploy the kuadrant operator

Create kuadrant-system namespace. Currently it has to be kuadrant-system as it is hardcoded in the consoleplugin service.

kubectl create ns kuadrant-system

To install the Kuadrant Operator, enter the following command to deploy catalog from this 46-consoleplugin branch:

kubectl apply -f - <<EOF
apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
  name: kuadrant-operator-catalog
  namespace: kuadrant-system
spec:
  sourceType: grpc
  image: quay.io/kuadrant/kuadrant-operator-catalog:46-consoleplugin
  displayName: Kuadrant Operators
  publisher: grpc
  updateStrategy:
    registryPoll:
      interval: 45m
---
apiVersion: operators.coreos.com/v1alpha1
kind: Subscription
metadata:
  name: kuadrant-operator
  namespace: kuadrant-system
spec:
  channel: preview
  installPlanApproval: Automatic
  name: kuadrant-operator
  source: kuadrant-operator-catalog
  sourceNamespace: kuadrant-system
---
kind: OperatorGroup
apiVersion: operators.coreos.com/v1
metadata:
  name: kuadrant
  namespace: kuadrant-system
spec:
  upgradeStrategy: Default
EOF

Wait for the Kuadrant Operators to be installed as follows:

kubectl get installplan -n kuadrant-system -o=jsonpath='{.items[0].status.phase}'

After some time, this command should return complete.

Installation done 🚀 Let's try it out

  • Enable Kuadrant's dynamic plugin

It may take some time to load the plugin. The first time it will be Disabled. It needs to be enabled.

In Home -> Overview , the status box has Dynamic Plugins. Click on it and then click on view all. Enable kuadrant-console plugin on the form as below:

Screenshot 2024-10-01 at 11-45-05 cluster · Console · Console plugins · Red Hat OpenShift

Once enabled, wait until the status says it is loaded, like in the snapshot below

Screenshot 2024-10-01 at 11-47-06 cluster · Console · Console plugins · Red Hat OpenShift

Refresh the openshift's console and a new entry Kuadrant in the left menu will show up

Screenshot 2024-10-01 at 11-51-39 cluster · Console · Console plugins · Red Hat OpenShift

If you check Overview and Policies it should be all empty (provided there is no gateway, httproute and kuadrant policy installed. Let's populate the cluster with some interesting resources.

  • Deploy resources

Apply the Kuadrant custom resource

kubectl -n kuadrant-system apply -f - <<EOF
---
apiVersion: kuadrant.io/v1beta1
kind: Kuadrant
metadata:
  name: kuadrant-sample
spec: {}
EOF

Add one gateway

kubectl -n kuadrant-system apply -f - <<EOF
---
apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
  labels:
    istio: ingressgateway
  name: kuadrant-ingressgateway
spec:
  gatewayClassName: istio
  listeners:
  - name: http
    port: 80
    protocol: HTTP
    allowedRoutes:
      namespaces:
        from: All
EOF

Add one HTTPRoute

kubectl -n kuadrant-system apply -f - <<EOF
---
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: toystore
spec:
  parentRefs:
  - name: kuadrant-ingressgateway
  hostnames:
  - api.toystore.com
  rules:
  - matches:
    - method: GET
      path:
        type: PathPrefix
        value: "/toys"
    backendRefs:
    - name: toystore
      port: 80
EOF

Add one RateLimitPolicy

kubectl -n kuadrant-system apply -f - <<EOF
---
apiVersion: kuadrant.io/v1beta2
kind: RateLimitPolicy
metadata:
  name: toystore
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  limits:
    "create-toy":
      rates:
      - limit: 5
        duration: 10
        unit: second
EOF

After that, the topology graph looks like this:

Screenshot 2024-10-01 at 12-07-33 Policy Topology

  • Check console plugin resource (cluster wide resource):
kubectl get consoleplugin kuadrant-console -o yaml

You should get something like:

apiVersion: console.openshift.io/v1
kind: ConsolePlugin
metadata:
  creationTimestamp: "2024-10-01T09:39:54Z"
  generation: 1
  labels:
    app: kuadrant-console
    app.kubernetes.io/component: kuadrant-console
    app.kubernetes.io/instance: kuadrant-console
    app.kubernetes.io/name: kuadrant-console
    app.kubernetes.io/part-of: kuadrant-console
  name: kuadrant-console
  resourceVersion: "130839645"
  uid: 5318cd76-993b-4597-b681-fe89257eadff
spec:
  backend:
    service:
      basePath: /
      name: kuadrant-console
      namespace: kuadrant-system
      port: 9443
    type: Service
  displayName: Kuadrant Console Plugin
  i18n:
    loadType: ""

Cleanup

kubectl delete consoleplugin kuadrant-console
kubectl delete namespace kuadrant-system

@eguzki eguzki added kind/enhancement New feature or request size/medium labels Sep 27, 2024
@eguzki eguzki self-assigned this Sep 27, 2024
Copy link

codecov bot commented Sep 27, 2024

Codecov Report

Attention: Patch coverage is 76.13636% with 84 lines in your changes missing coverage. Please review.

Project coverage is 81.18%. Comparing base (ece13e8) to head (8121712).
Report is 224 commits behind head on main.

Files with missing lines Patch % Lines
controllers/fake/manager.go 22.22% 28 Missing ⚠️
pkg/library/reconcilers/deployment.go 0.00% 19 Missing ⚠️
controllers/consoleplugin_reconciler.go 81.25% 8 Missing and 4 partials ⚠️
controllers/state_of_the_world.go 26.66% 9 Missing and 2 partials ⚠️
controllers/fake/event_recorder.go 0.00% 6 Missing ⚠️
controllers/fake/api_reader.go 0.00% 4 Missing ⚠️
pkg/library/gatewayapi/utils.go 25.00% 0 Missing and 3 partials ⚠️
pkg/library/utils/crd.go 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #884      +/-   ##
==========================================
+ Coverage   80.20%   81.18%   +0.97%     
==========================================
  Files          64      115      +51     
  Lines        4492     7420    +2928     
==========================================
+ Hits         3603     6024    +2421     
- Misses        600      967     +367     
- Partials      289      429     +140     
Flag Coverage Δ
bare-k8s-integration 8.06% <7.53%> (?)
controllers-integration 70.89% <8.21%> (?)
envoygateway-integration 48.14% <8.21%> (?)
gatewayapi-integration 13.13% <7.53%> (?)
integration ?
istio-integration 51.17% <7.53%> (?)
unit 30.77% <69.51%> (+0.73%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
api/v1beta1 (u) 90.90% <100.00%> (+19.48%) ⬆️
api/v1beta2 (u) 86.61% <81.94%> (-4.82%) ⬇️
pkg/common (u) 88.13% <ø> (-0.70%) ⬇️
pkg/istio (u) 71.51% <ø> (-2.40%) ⬇️
pkg/log (u) 94.73% <ø> (ø)
pkg/reconcilers (u) ∅ <ø> (∅)
pkg/rlptools (u) 85.55% <ø> (+6.09%) ⬆️
controllers (i) 82.01% <84.52%> (+5.21%) ⬆️
Files with missing lines Coverage Δ
controllers/fake/manager_builder.go 100.00% <100.00%> (ø)
controllers/test_common.go 100.00% <100.00%> (ø)
controllers/topology_reconciler.go 76.31% <100.00%> (ø)
pkg/envoygateway/utils.go 71.42% <100.00%> (ø)
pkg/istio/utils.go 79.16% <100.00%> (ø)
pkg/openshift/consoleplugin/common.go 100.00% <100.00%> (ø)
pkg/openshift/consoleplugin/consoleplugin.go 100.00% <100.00%> (ø)
pkg/openshift/consoleplugin/deployment.go 100.00% <100.00%> (ø)
pkg/openshift/consoleplugin/nginx_configmap.go 100.00% <100.00%> (ø)
pkg/openshift/consoleplugin/service.go 100.00% <100.00%> (ø)
... and 9 more

... and 59 files with indirect coverage changes

@@ -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

@jasonmadigan
Copy link
Member

Merged the docs

@eguzki eguzki marked this pull request as ready for review October 3, 2024 08:39
@eguzki eguzki requested a review from KevFan October 3, 2024 09:16
@jasonmadigan
Copy link
Member

LGTM

Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
Makefile Outdated
@@ -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 ^^

if !topologyExists {
utils.TagObjectToDelete(service)
}
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

@KevFan
Copy link
Contributor

KevFan commented Oct 7, 2024

Verified steps on a local CRC cluster on Openshift version 4.16.7 👍

@eguzki eguzki changed the title consoleplugin task consoleplugin reconciler Oct 7, 2024
Copy link
Contributor

@KevFan KevFan left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@eguzki eguzki merged commit bd043cd into main Oct 8, 2024
31 of 32 checks passed
@eguzki eguzki deleted the 46-consoleplugin branch October 8, 2024 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request size/medium
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Plugin installation + docs
4 participants