-
Notifications
You must be signed in to change notification settings - Fork 33
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
Changes from 9 commits
14bb1c1
7206036
c8fc7e1
33fafe2
f44250e
75c155d
857986c
616d470
e5bcefd
dcdb0cc
b9bf7d7
8121712
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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}') | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
{
"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"
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The command line 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 I will try to enhance makefile command to handle error when the namespace is not available. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, yeah adding There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I think that for
But we can leave this for other PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||||||
|
@@ -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) | ||||||
|
@@ -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 | ||||||
|
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, | ||
} | ||
} | ||
|
||
func (r *ConsolePluginReconciler) Subscription() *controller.Subscription { | ||
return &controller.Subscription{ | ||
ReconcileFunc: r.Run, | ||
Events: []controller.ResourceEventMatcher{ | ||
{Kind: ptr.To(openshift.ConsolePluginGVK.GroupKind())}, | ||
{ | ||
Kind: ptr.To(ConfigMapGroupKind), | ||
ObjectNamespace: r.namespace, | ||
ObjectName: TopologyConfigMapName, | ||
EventType: ptr.To(controller.CreateEvent), | ||
}, | ||
{ | ||
Kind: ptr.To(ConfigMapGroupKind), | ||
ObjectNamespace: r.namespace, | ||
ObjectName: TopologyConfigMapName, | ||
EventType: ptr.To(controller.DeleteEvent), | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
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) | ||
|
||
existingTopologyConfigMaps := topology.Objects().Items(func(object machinery.Object) bool { | ||
return object.GetName() == TopologyConfigMapName && object.GetNamespace() == r.namespace && object.GroupVersionKind().Kind == ConfigMapGroupKind.Kind | ||
}) | ||
|
||
topologyExists := len(existingTopologyConfigMaps) > 0 | ||
|
||
// Service | ||
service := consoleplugin.Service(r.namespace) | ||
if !topologyExists { | ||
utils.TagObjectToDelete(service) | ||
} | ||
err := r.ReconcileResource(ctx, &corev1.Service{}, service, reconcilers.CreateOnlyMutator) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I'm fine with leaving it to be addressed later 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
} | ||
|
||
// Deployment | ||
deployment := consoleplugin.Deployment(r.namespace, ConsolePluginImageURL) | ||
deploymentMutators := make([]reconcilers.DeploymentMutateFn, 0) | ||
deploymentMutators = append(deploymentMutators, reconcilers.DeploymentImageMutator) | ||
if !topologyExists { | ||
utils.TagObjectToDelete(deployment) | ||
} | ||
err = r.ReconcileResource(ctx, &appsv1.Deployment{}, deployment, reconcilers.DeploymentMutator(deploymentMutators...)) | ||
if err != nil { | ||
logger.Error(err, "reconciling deployment") | ||
return err | ||
} | ||
|
||
// Nginx ConfigMap | ||
nginxConfigMap := consoleplugin.NginxConfigMap(r.namespace) | ||
if !topologyExists { | ||
utils.TagObjectToDelete(nginxConfigMap) | ||
} | ||
err = r.ReconcileResource(ctx, &corev1.ConfigMap{}, nginxConfigMap, reconcilers.CreateOnlyMutator) | ||
if err != nil { | ||
logger.Error(err, "reconciling nginx configmap") | ||
return err | ||
} | ||
|
||
// ConsolePlugin | ||
consolePlugin := consoleplugin.ConsolePlugin(r.namespace) | ||
if !topologyExists { | ||
utils.TagObjectToDelete(consolePlugin) | ||
} | ||
err = r.ReconcileResource(ctx, &consolev1.ConsolePlugin{}, consolePlugin, reconcilers.CreateOnlyMutator) | ||
if err != nil { | ||
logger.Error(err, "reconciling consoleplugin") | ||
return err | ||
} | ||
|
||
logger.V(1).Info("task ended") | ||
return nil | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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