-
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
Add support for project sail #323
Changes from all commits
19577a6
64405b8
d285fc2
650d156
1570b12
57ed8b9
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 |
---|---|---|
|
@@ -8,12 +8,9 @@ metadata: | |
spec: | ||
gatewayClassName: istio | ||
listeners: | ||
- name: default | ||
- name: http | ||
port: 80 | ||
protocol: HTTP | ||
allowedRoutes: | ||
namespaces: | ||
from: All | ||
addresses: | ||
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. glad we can remove this,but does it cause any issues with other user-guides? 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. On a sail install this part of the spec gets populated when it creates the loadbalancer service so it should be present anyway. I'll double check that I didn't miss any user guides though. |
||
- value: istio-ingressgateway.istio-system.svc.cluster.local | ||
type: Hostname |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
apiVersion: operator.istio.io/v1alpha1 | ||
kind: Istio | ||
metadata: | ||
name: istiocontrolplane | ||
namespace: istio-system | ||
spec: | ||
version: v1.20.0 | ||
# Disable autoscaling to reduce dev resources | ||
values: | ||
pilot: | ||
autoscaleEnabled: false | ||
rawValues: | ||
gateways: | ||
istio-ingressgateway: | ||
autoscaleEnabled: false | ||
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. be good to get a couple of comments as to what it does or why its needed? 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 was just to match our original local deploy configuration but isn't necessary and could be removed https://github.com/Kuadrant/kuadrant-operator/blob/main/config/dependencies/istio/istio-operator.yaml#L48-L54 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
--- | ||
namespace: istio-system | ||
resources: | ||
- github.com/maistra/istio-operator/config/default?ref=maistra-3.0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ | |
apimeta "k8s.io/apimachinery/pkg/api/meta" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/apimachinery/pkg/runtime" | ||
istiov1alpha1 "maistra.io/istio-operator/api/v1alpha1" | ||
ctrl "sigs.k8s.io/controller-runtime" | ||
"sigs.k8s.io/controller-runtime/pkg/client" | ||
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" | ||
|
@@ -70,6 +71,7 @@ | |
//+kubebuilder:rbac:groups="gateway.networking.k8s.io",resources=httproutes,verbs=get;list;patch;update;watch | ||
//+kubebuilder:rbac:groups=operator.authorino.kuadrant.io,resources=authorinos,verbs=get;list;watch;create;update;delete;patch | ||
//+kubebuilder:rbac:groups=install.istio.io,resources=istiooperators,verbs=get;list;watch;create;update;patch | ||
//+kubebuilder:rbac:groups=operator.istio.io,resources=istios,verbs=get;list;watch;create;update;patch | ||
//+kubebuilder:rbac:groups=maistra.io,resources=servicemeshcontrolplanes,verbs=get;list;watch;update;use;patch | ||
//+kubebuilder:rbac:groups=maistra.io,resources=servicemeshmembers,verbs=get;list;watch;create;update;delete;patch | ||
|
||
|
@@ -320,23 +322,32 @@ | |
var configsToUpdate []common.ConfigWrapper | ||
|
||
iop := &iopv1alpha1.IstioOperator{} | ||
iopKey := client.ObjectKey{Name: controlPlaneProviderName(), Namespace: controlPlaneProviderNamespace()} | ||
if err := r.GetResource(ctx, iopKey, iop); err != nil { | ||
logger.V(1).Info("failed to get istiooperator object", "key", iopKey, "err", err) | ||
if apimeta.IsNoMatchError(err) { | ||
// return nil and nil if there's no istiooperator CRD, means istio is not installed | ||
return nil, nil | ||
} else if !apierrors.IsNotFound(err) { | ||
// return nil and err if there's an error other than not found (no istiooperator CR) | ||
return nil, err | ||
} | ||
} else { | ||
istKey := client.ObjectKey{Name: controlPlaneProviderName(), Namespace: controlPlaneProviderNamespace()} | ||
err := r.GetResource(ctx, istKey, iop) | ||
if err == nil || apierrors.IsNotFound(err) { | ||
configsToUpdate = append(configsToUpdate, istio.NewOperatorWrapper(iop)) | ||
} else if !apimeta.IsNoMatchError(err) { | ||
logger.V(1).Info("failed to get istiooperator object", "key", istKey, "err", err) | ||
return nil, err | ||
} else { | ||
// Error is NoMatchError so check for Istio CR instead | ||
ist := &istiov1alpha1.Istio{} | ||
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. Is this CR watched by the istio operator too? 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. Yep it is (the project sail install / not istioctl install which still uses |
||
if err := r.GetResource(ctx, istKey, ist); err != nil { | ||
logger.V(1).Info("failed to get istio object", "key", istKey, "err", err) | ||
if apimeta.IsNoMatchError(err) { | ||
// return nil and nil if there's no istiooperator or istio CR | ||
return nil, nil | ||
} else if !apierrors.IsNotFound(err) { | ||
// return nil and err if there's an error other than not found (no istio CR) | ||
return nil, err | ||
} | ||
} | ||
configsToUpdate = append(configsToUpdate, istio.NewSailWrapper(ist)) | ||
} | ||
|
||
istioConfigMap := &corev1.ConfigMap{} | ||
if err := r.GetResource(ctx, client.ObjectKey{Name: controlPlaneConfigMapName(), Namespace: controlPlaneProviderNamespace()}, istioConfigMap); err != nil { | ||
logger.V(1).Info("failed to get istio configMap", "key", iopKey, "err", err) | ||
logger.V(1).Info("failed to get istio configMap", "key", istKey, "err", err) | ||
return configsToUpdate, err | ||
} | ||
configsToUpdate = append(configsToUpdate, istio.NewConfigMapWrapper(istioConfigMap)) | ||
|
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.
I don't mind the name change, but any specific reason to change it?
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.
This was just to match the naming when retrieving the port from the gateway as used in the istio docs - no real reason though we can leave it as default and I can update the exports in each of the guides