Skip to content
This repository has been archived by the owner on Dec 3, 2019. It is now read-only.

Commit

Permalink
Refactor things to clarify which functions are for which platform
Browse files Browse the repository at this point in the history 
This also introduces the idea of configuring the resources *before*
they're applied, which in hindsight makes more sense, i.e. make the
first configmap the controllers see the correct one.

We may want to do the same for the config in the Install CR, but
because I'm still unsure whether having the config in there at all is
a good idea, I'm keeping it post-install, mostly because the logging
is set up to show previous values of whatever it overwrites. Doing
that pre-install would require manifestival enhancements.

Another thing pre-install config solves is swimming upstream against
the controller-runtime caching client, which occasionally fails to
find newly-created resources as the informer doesn't yet know about
them.

See kubernetes-sigs/controller-runtime#180
for more info.
  • Loading branch information
@jcrossley3
jcrossley3 committed May 12, 2019
1 parent 14d83d7 commit aa755a4
Show file tree
Hide file tree
Showing 5 changed files with 198 additions and 140 deletions.
9 changes: 9 additions & 0 deletions pkg/controller/install/add_minikube.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package install

import (
"github.com/openshift-knative/knative-serving-operator/pkg/controller/install/minikube"
)

func init() {
platformFuncs = append(platformFuncs, minikube.Configure)
}
9 changes: 9 additions & 0 deletions pkg/controller/install/add_openshift.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package install

import (
"github.com/openshift-knative/knative-serving-operator/pkg/controller/install/openshift"
)

func init() {
platformFuncs = append(platformFuncs, openshift.Configure)
}
172 changes: 32 additions & 140 deletions pkg/controller/install/install_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,15 @@ import (
"context"
"flag"
"fmt"
"strings"

mf "github.com/jcrossley3/manifestival"
servingv1alpha1 "github.com/openshift-knative/knative-serving-operator/pkg/apis/serving/v1alpha1"
"github.com/openshift-knative/knative-serving-operator/version"
configv1 "github.com/openshift/api/config/v1"

"github.com/operator-framework/operator-sdk/pkg/predicate"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/handler"
Expand All @@ -35,6 +30,8 @@ var (
installNs = flag.String("install-ns", "",
"The namespace in which to create an Install resource, if none exist")
log = logf.Log.WithName("controller_install")
// Platform-specific configuration via manifestival transformations
platformFuncs []func(client.Client, *runtime.Scheme) []mf.Transformer
)

// Add creates a new Install Controller and adds it to the Manager. The Manager will set fields on the Controller
Expand Down Expand Up @@ -96,22 +93,19 @@ func (r *ReconcileInstall) Reconcile(request reconcile.Request) (reconcile.Resul

// Fetch the Install instance
instance := &servingv1alpha1.Install{}
err := r.client.Get(context.TODO(), request.NamespacedName, instance)
if err != nil {
if err := r.client.Get(context.TODO(), request.NamespacedName, instance); err != nil {
if errors.IsNotFound(err) {
r.config.DeleteAll()
return reconcile.Result{}, nil
}
// Error reading the object - requeue the request.
return reconcile.Result{}, err
}

stages := []func(*servingv1alpha1.Install) error{
r.transform,
r.install,
r.deleteObsoleteResources,
r.checkForMinikube,
r.checkForOpenShift,
r.configure,
r.configure, // TODO: move to transform?
}

for _, stage := range stages {
Expand All @@ -123,24 +117,24 @@ func (r *ReconcileInstall) Reconcile(request reconcile.Request) (reconcile.Resul
return reconcile.Result{}, nil
}

// Apply the embedded resources
func (r *ReconcileInstall) install(instance *servingv1alpha1.Install) error {
// Transform resources as appropriate
fns := []mf.Transformer{mf.InjectOwner(instance), addPolicyRules}
// Transform resources as appropriate for the spec and platform
func (r *ReconcileInstall) transform(instance *servingv1alpha1.Install) error {
fns := []mf.Transformer{mf.InjectOwner(instance)}
if len(instance.Spec.Namespace) > 0 {
fns = append(fns, mf.InjectNamespace(instance.Spec.Namespace))
}
for _, f := range platformFuncs {
fns = append(fns, f(r.client, r.scheme)...)
}
r.config.Transform(fns...)
return nil
}

if instance.Status.Version == version.Version {
// we've already successfully applied our YAML
return nil
}
// Apply the resources in the YAML file
// Apply the embedded resources
func (r *ReconcileInstall) install(instance *servingv1alpha1.Install) error {
if err := r.config.ApplyAll(); err != nil {
return err
}

// Update status
instance.Status.Resources = r.config.Resources
instance.Status.Version = version.Version
Expand Down Expand Up @@ -169,6 +163,22 @@ func (r *ReconcileInstall) configure(instance *servingv1alpha1.Install) error {
return nil
}

// Set some data in a configmap, only overwriting common keys
func (r *ReconcileInstall) updateConfigMap(cm *unstructured.Unstructured, data map[string]string) error {
for k, v := range data {
message := []interface{}{"map", cm.GetName(), k, v}
if x, found, _ := unstructured.NestedString(cm.Object, "data", k); found {
if v == x {
continue
}
message = append(message, "previous", x)
}
log.Info("Setting", message...)
unstructured.SetNestedField(cm.Object, v, "data", k)
}
return r.config.Apply(cm)
}

// Delete obsolete istio-system resources, if any
func (r *ReconcileInstall) deleteObsoleteResources(instance *servingv1alpha1.Install) error {
resource := &unstructured.Unstructured{}
Expand All @@ -189,101 +199,7 @@ func (r *ReconcileInstall) deleteObsoleteResources(instance *servingv1alpha1.Ins
return r.config.Delete(resource)
}

// Set some data in a configmap, only overwriting common keys
func (r *ReconcileInstall) updateConfigMap(cm *unstructured.Unstructured, data map[string]string) error {
for k, v := range data {
values := []interface{}{"map", cm.GetName(), k, v}
if x, found, _ := unstructured.NestedString(cm.Object, "data", k); found {
if v == x {
continue
}
values = append(values, "previous", x)
}
log.Info("Setting", values...)
unstructured.SetNestedField(cm.Object, v, "data", k)
}
return r.config.Apply(cm)

}

// Configure minikube if we're soaking in it
func (r *ReconcileInstall) checkForMinikube(instance *servingv1alpha1.Install) error {
node := &v1.Node{}
err := r.client.Get(context.TODO(), types.NamespacedName{Name: "minikube"}, node)
if err != nil {
if errors.IsNotFound(err) {
return nil // not running on minikube!
}
return err
}
cm, err := r.config.Get(r.config.Find("v1", "ConfigMap", "config-network"))
if err != nil {
return err
}
if cm == nil {
log.Error(err, "Missing ConfigMap", "name", "config-network")
return nil // no sense in trying if the CM is gone
}
log.Info("Detected minikube; checking egress")
data := map[string]string{"istio.sidecar.includeOutboundIPRanges": "10.0.0.1/24"}
return r.updateConfigMap(cm, data)
}

// Configure OpenShift if we're soaking in it
func (r *ReconcileInstall) checkForOpenShift(instance *servingv1alpha1.Install) error {
tasks := []func(*servingv1alpha1.Install) error{
r.configureIngressForOpenShift,
r.configureEgressForOpenShift,
}
for _, task := range tasks {
if err := task(instance); err != nil {
return err
}
}
return nil
}

func (r *ReconcileInstall) configureIngressForOpenShift(instance *servingv1alpha1.Install) error {
ingressConfig := &configv1.Ingress{}
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, ingressConfig); err != nil {
if meta.IsNoMatchError(err) {
log.V(1).Info("OpenShift Ingress Config is not available.")
return nil // not on OpenShift
}
return err
}
domain := ingressConfig.Spec.Domain
// If domain is available, update config-domain config map
if len(domain) > 0 {
log.Info("OpenShift Ingress Config is available", "Domain", domain)
cm := r.config.Find("v1", "ConfigMap", "config-domain")
data := map[string]string{domain: ""}
return r.updateConfigMap(cm, data)
}
return nil
}

// Set istio.sidecar.includeOutboundIPRanges property with service network
func (r *ReconcileInstall) configureEgressForOpenShift(instance *servingv1alpha1.Install) error {
networkConfig := &configv1.Network{}
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, networkConfig); err != nil {
if meta.IsNoMatchError(err) {
log.V(1).Info("OpenShift Network Config is not available.")
return nil // not on OpenShift
}
return err
}
serviceNetwork := strings.Join(networkConfig.Spec.ServiceNetwork, ",")
// If service network is available, update config-network config map
if len(serviceNetwork) > 0 {
log.Info("OpenShift Network Config is available", "ServiceNetwork", serviceNetwork)
cm := r.config.Find("v1", "ConfigMap", "config-network")
data := map[string]string{"istio.sidecar.includeOutboundIPRanges": serviceNetwork}
return r.updateConfigMap(cm, data)
}
return nil
}

// This may or may not be a good idea
func autoInstall(c client.Client, ns string) (err error) {
const path = "deploy/crds/serving_v1alpha1_install_cr.yaml"
log.Info("Automatic Install requested", "namespace", ns)
Expand All @@ -306,27 +222,3 @@ func autoInstall(c client.Client, ns string) (err error) {
}
return err
}

// TODO: These are addressed in master and shouldn't be required for 0.6.0
func addPolicyRules(u *unstructured.Unstructured) *unstructured.Unstructured {
if u.GetKind() == "ClusterRole" && u.GetName() == "knative-serving-core" {
field, _, _ := unstructured.NestedFieldNoCopy(u.Object, "rules")
var rules []interface{} = field.([]interface{})
for _, rule := range rules {
m := rule.(map[string]interface{})
for _, group := range m["apiGroups"].([]interface{}) {
if group == "apps" {
m["resources"] = append(m["resources"].([]interface{}), "deployments/finalizers")
}
}
}
// Required to open privileged ports in OpenShift
unstructured.SetNestedField(u.Object, append(rules, map[string]interface{}{
"apiGroups": []interface{}{"security.openshift.io"},
"verbs": []interface{}{"use"},
"resources": []interface{}{"securitycontextconstraints"},
"resourceNames": []interface{}{"privileged", "anyuid"},
}), "rules")
}
return u
}
38 changes: 38 additions & 0 deletions pkg/controller/install/minikube/minikube.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package minikube

import (
"context"

mf "github.com/jcrossley3/manifestival"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
)

var log = logf.Log.WithName("minikube")

// Configure minikube if we're soaking in it
func Configure(c client.Client, _ *runtime.Scheme) []mf.Transformer {
node := &v1.Node{}
if err := c.Get(context.TODO(), types.NamespacedName{Name: "minikube"}, node); err != nil {
if !errors.IsNotFound(err) {
log.Error(err, "Unable to query for minikube node")
}
return nil // not running on minikube
}
return []mf.Transformer{egress}
}

func egress(u *unstructured.Unstructured) *unstructured.Unstructured {
if u.GetKind() == "ConfigMap" && u.GetName() == "config-network" {
k, v := "istio.sidecar.includeOutboundIPRanges", "10.0.0.1/24"
log.Info("Setting egress", k, v)
unstructured.SetNestedField(u.Object, v, "data", k)
}
return u
}
Loading

0 comments on commit aa755a4

Please sign in to comment.