Skip to content

Commit 42dbcf8

Browse files
danehansdanehans
committed
Refactors PR to focus on http/https/no proxy reconciliation
1 parent 6b189c8 commit 42dbcf8

6 files changed

Lines changed: 71 additions & 390 deletions

File tree

‎Gopkg.lock‎

Lines changed: 0 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/controller/proxyconfig/controller.go‎

Lines changed: 60 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,15 @@ import (
88
configv1 "github.com/openshift/api/config/v1"
99
"github.com/openshift/cluster-network-operator/pkg/controller/statusmanager"
1010
"github.com/openshift/cluster-network-operator/pkg/names"
11-
"github.com/openshift/cluster-network-operator/pkg/util/validation"
1211

1312
corev1 "k8s.io/api/core/v1"
1413
apierrors "k8s.io/apimachinery/pkg/api/errors"
1514
"k8s.io/apimachinery/pkg/runtime"
1615
"k8s.io/apimachinery/pkg/types"
1716
"sigs.k8s.io/controller-runtime/pkg/client"
1817
"sigs.k8s.io/controller-runtime/pkg/controller"
19-
"sigs.k8s.io/controller-runtime/pkg/event"
2018
"sigs.k8s.io/controller-runtime/pkg/handler"
2119
"sigs.k8s.io/controller-runtime/pkg/manager"
22-
"sigs.k8s.io/controller-runtime/pkg/predicate"
2320
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2421
"sigs.k8s.io/controller-runtime/pkg/source"
2522
)
@@ -51,33 +48,6 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
5148
return err
5249
}
5350

54-
// We only care about a configmap source with a specific name/namespace,
55-
// so filter events before they are provided to the controller event handlers.
56-
pred := predicate.Funcs{
57-
UpdateFunc: func(e event.UpdateEvent) bool {
58-
return e.MetaOld.GetName() == names.TRUST_BUNDLE_CONFIGMAP &&
59-
e.MetaOld.GetNamespace() == names.TRUST_BUNDLE_CONFIGMAP_NS
60-
},
61-
DeleteFunc: func(e event.DeleteEvent) bool {
62-
return e.Meta.GetName() == names.TRUST_BUNDLE_CONFIGMAP &&
63-
e.Meta.GetNamespace() == names.TRUST_BUNDLE_CONFIGMAP_NS
64-
},
65-
CreateFunc: func(e event.CreateEvent) bool {
66-
return e.Meta.GetName() == names.TRUST_BUNDLE_CONFIGMAP &&
67-
e.Meta.GetNamespace() == names.TRUST_BUNDLE_CONFIGMAP_NS
68-
},
69-
GenericFunc: func(e event.GenericEvent) bool {
70-
return e.Meta.GetName() == names.TRUST_BUNDLE_CONFIGMAP &&
71-
e.Meta.GetNamespace() == names.TRUST_BUNDLE_CONFIGMAP_NS
72-
},
73-
}
74-
75-
// Watch for changes to the trust bundle configmap.
76-
err = c.Watch(&source.Kind{Type: &corev1.ConfigMap{}}, &handler.EnqueueRequestForObject{}, pred)
77-
if err != nil {
78-
return err
79-
}
80-
8151
// Watch for changes to the proxy resource.
8252
err = c.Watch(&source.Kind{Type: &configv1.Proxy{}}, &handler.EnqueueRequestForObject{})
8353
if err != nil {
@@ -97,94 +67,72 @@ type ReconcileProxyConfig struct {
9767
}
9868

9969
// Reconcile expects request to refer to a proxy object named "cluster"
100-
// in the default namespace or to a configmap object named
101-
// "trusted-ca-bundle" in namespace "openshift-config-managed", and will
102-
// ensure the proxy object is in the desired state.
70+
// in the default namespace and will ensure the proxy object is in the
71+
// desired state.
10372
func (r *ReconcileProxyConfig) Reconcile(request reconcile.Request) (reconcile.Result, error) {
104-
switch {
105-
case request.NamespacedName == names.Proxy():
106-
// Collect required config objects for proxy reconciliation.
107-
proxyConfig := &configv1.Proxy{}
108-
infraConfig := &configv1.Infrastructure{}
109-
netConfig := &configv1.Network{}
110-
clusterConfig := &corev1.ConfigMap{}
111-
112-
log.Printf("Reconciling proxy: %s\n", request.Name)
113-
err := r.client.Get(context.TODO(), request.NamespacedName, proxyConfig)
114-
if err != nil {
115-
if apierrors.IsNotFound(err) {
116-
// Request object not found, could have been deleted after reconcile request.
117-
// Return and don't requeue
118-
log.Println("proxy not found; reconciliation will be skipped", "request", request)
119-
return reconcile.Result{}, nil
120-
}
121-
// Error reading the object - requeue the request.
122-
return reconcile.Result{}, fmt.Errorf("failed to get proxy %q: %v", request, err)
73+
log.Printf("Reconciling proxy '%s'", request.Name)
74+
proxyConfig := &configv1.Proxy{}
75+
err := r.client.Get(context.TODO(), request.NamespacedName, proxyConfig)
76+
if err != nil {
77+
if apierrors.IsNotFound(err) {
78+
// Request object not found, could have been deleted after reconcile request.
79+
// Return and don't requeue
80+
log.Printf("proxy '%s' not found; reconciliation will be skipped", request.Name)
81+
return reconcile.Result{}, nil
12382
}
83+
// Error reading the object - requeue the request.
84+
return reconcile.Result{}, fmt.Errorf("failed to get proxy '%s': %v", request.Name, err)
85+
}
12486

125-
// A nil proxy is generated by upgrades and installs not requiring a proxy.
126-
validate := true
127-
if !isSpecHTTPProxySet(&proxyConfig.Spec) && !isSpecHTTPSProxySet(&proxyConfig.Spec) {
128-
log.Printf("httpProxy and httpsProxy not defined; validation will be skipped for proxy: %s\n",
129-
request.Name)
130-
validate = false
131-
}
87+
// A nil proxy is generated by upgrades and installs not requiring a proxy.
88+
if !isSpecHTTPProxySet(&proxyConfig.Spec) &&
89+
!isSpecHTTPSProxySet(&proxyConfig.Spec) &&
90+
!isSpecNoProxySet(&proxyConfig.Spec) {
91+
log.Printf("httpProxy, httpsProxy and noProxy not defined for proxy '%s'; validation will be skipped",
92+
request.Name)
93+
}
13294

133-
// Only proceed if the required config objects can be collected.
134-
if validate {
135-
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, infraConfig); err != nil {
136-
return reconcile.Result{}, fmt.Errorf("failed to get infrastructure config 'cluster': %v", err)
137-
}
138-
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, netConfig); err != nil {
139-
log.Printf("failed to get network config 'cluster': %v", err)
140-
return reconcile.Result{}, err
141-
}
142-
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "cluster-config-v1", Namespace: "kube-system"}, clusterConfig); err != nil {
143-
log.Printf("failed to get configmap 'cluster': %v", err)
144-
return reconcile.Result{}, err
145-
}
146-
if err := r.ValidateProxyConfig(&proxyConfig.Spec); err != nil {
147-
log.Printf("Failed to validate Proxy.Spec: %v", err)
148-
r.status.SetDegraded(statusmanager.ProxyConfig, "InvalidProxyConfig",
149-
fmt.Sprintf("The proxy configuration is invalid (%v). Use 'oc edit proxy.config.openshift.io cluster' to fix.", err))
150-
return reconcile.Result{}, nil
151-
}
152-
}
95+
// Only proceed if the required config objects can be collected.
96+
infraConfig := &configv1.Infrastructure{}
97+
netConfig := &configv1.Network{}
98+
clusterCfgMap := &corev1.ConfigMap{}
99+
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: names.CLUSTER_CONFIG}, infraConfig); err != nil {
100+
log.Printf("failed to get infrastructure config '%s': %v", names.CLUSTER_CONFIG, err)
101+
r.status.SetDegraded(statusmanager.ProxyConfig, "InfraConfigError",
102+
fmt.Sprintf("Error getting infrastructure config %s: %v.", names.CLUSTER_CONFIG, err))
103+
return reconcile.Result{}, nil
104+
}
105+
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: names.CLUSTER_CONFIG}, netConfig); err != nil {
106+
log.Printf("failed to get network config '%s': %v", names.CLUSTER_CONFIG, err)
107+
r.status.SetDegraded(statusmanager.ProxyConfig, "NetworkConfigError",
108+
fmt.Sprintf("Error getting network config '%s': %v.", names.CLUSTER_CONFIG, err))
109+
return reconcile.Result{}, nil
110+
}
111+
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "cluster-config-v1", Namespace: "kube-system"},
112+
clusterCfgMap); err != nil {
113+
log.Printf("failed to get configmap '%s/%s': %v", clusterCfgMap.Namespace, clusterCfgMap.Name, err)
114+
r.status.SetDegraded(statusmanager.ProxyConfig, "ClusterConfigError",
115+
fmt.Sprintf("Error getting cluster config configmap '%s/%s': %v.", clusterCfgMap.Namespace,
116+
clusterCfgMap.Name, err))
117+
return reconcile.Result{}, nil
118+
}
119+
if err := r.ValidateProxyConfig(&proxyConfig.Spec); err != nil {
120+
log.Printf("Failed to validate proxy '%s': %v", proxyConfig.Name, err)
121+
r.status.SetDegraded(statusmanager.ProxyConfig, "InvalidProxyConfig",
122+
fmt.Sprintf("The configuration is invalid for proxy '%s' (%v). "+
123+
"Use 'oc edit proxy.config.openshift.io %s' to fix.", proxyConfig.Name, proxyConfig.Name, err))
124+
return reconcile.Result{}, nil
125+
}
153126

154-
// Update proxy status.
155-
if err := r.syncProxyStatus(proxyConfig, infraConfig, netConfig, clusterConfig); err != nil {
156-
log.Printf("Could not sync proxy status: %v", err)
157-
r.status.SetDegraded(statusmanager.ProxyConfig, "StatusError",
158-
fmt.Sprintf("Could not update proxy configuration status: %v", err))
159-
return reconcile.Result{}, err
160-
}
161-
log.Printf("Reconciling proxy: %s complete\n", request.Name)
162-
case request.NamespacedName == names.TrustBundleConfigMap():
163-
cfgMap := &corev1.ConfigMap{}
164-
log.Printf("Reconciling configmap: %s/%s\n", request.Namespace, request.Name)
165-
err := r.client.Get(context.TODO(), request.NamespacedName, cfgMap)
166-
if err != nil {
167-
if apierrors.IsNotFound(err) {
168-
// Request object not found, could have been deleted after reconcile request.
169-
// Return and don't requeue
170-
log.Println("configmap not found; reconciliation will be skipped", "request", request)
171-
return reconcile.Result{}, nil
172-
}
173-
// Error reading the object - requeue the request.
174-
return reconcile.Result{}, fmt.Errorf("failed to get configmap %q: %v", request, err)
175-
}
176-
if _, _, err := validation.TrustBundleConfigMap(cfgMap); err != nil {
177-
log.Printf("Failed to validate configmap: %v", err)
178-
r.status.SetDegraded(statusmanager.ProxyConfig, "InvalidTrustedCAConfigMap",
179-
fmt.Sprintf("The configmap is invalid (%v). Use 'oc edit configmap %s -n %s' to fix.", err,
180-
request.Name, request.Namespace))
181-
return reconcile.Result{}, nil
182-
}
183-
log.Printf("Reconciling configmap: %s/%s complete\n", request.Namespace, request.Name)
184-
default:
185-
// unknown object
186-
log.Println("Ignoring unknown object, reconciliation will be skipped", "request", request)
127+
// Update proxy status.
128+
if err := r.syncProxyStatus(proxyConfig, infraConfig, netConfig, clusterCfgMap); err != nil {
129+
log.Printf("Could not sync proxy '%s' status: %v", proxyConfig.Name, err)
130+
r.status.SetDegraded(statusmanager.ProxyConfig, "StatusError",
131+
fmt.Sprintf("Could not update proxy '%s' status: %v", proxyConfig.Name, err))
132+
return reconcile.Result{}, err
187133
}
134+
log.Printf("Reconciling proxy '%s' complete", request.Name)
135+
188136
r.status.SetNotDegraded(statusmanager.ProxyConfig)
189137

190138
return reconcile.Result{}, nil
@@ -206,20 +154,3 @@ func isSpecHTTPSProxySet(proxyConfig *configv1.ProxySpec) bool {
206154
func isSpecNoProxySet(proxyConfig *configv1.ProxySpec) bool {
207155
return len(proxyConfig.NoProxy) > 0
208156
}
209-
210-
// isSpecTrustedCASet returns true if spec.trustedCA of proxyConfig is set.
211-
func isSpecTrustedCASet(proxyConfig *configv1.ProxySpec) bool {
212-
return len(proxyConfig.TrustedCA.Name) > 0
213-
}
214-
215-
// isTrustedCAConfigMap returns true if the ConfigMap name in
216-
// spec.trustedCA is "proxy-ca-bundle".
217-
func isTrustedCAConfigMap(proxyConfig *configv1.ProxySpec) bool {
218-
return proxyConfig.TrustedCA.Name == names.TRUST_BUNDLE_CONFIGMAP
219-
}
220-
221-
// isSpecReadinessEndpoints returns true if spec.readinessEndpoints of
222-
// proxyConfig is set.
223-
func isSpecReadinessEndpoints(proxyConfig *configv1.ProxySpec) bool {
224-
return len(proxyConfig.ReadinessEndpoints) > 0
225-
}

‎pkg/controller/proxyconfig/status.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
)
1717

1818
// syncProxyStatus computes the current status of proxy and
19-
// updates status if any changes since last sync.
19+
// updates status of any changes since last sync.
2020
func (r *ReconcileProxyConfig) syncProxyStatus(proxy *configv1.Proxy, infra *configv1.Infrastructure, network *configv1.Network, cluster *corev1.ConfigMap) error {
2121
var err error
2222
var noProxy string
@@ -87,10 +87,8 @@ func mergeUserSystemNoProxy(proxy *configv1.Proxy, infra *configv1.Infrastructur
8787
}
8888

8989
switch infra.Status.PlatformStatus.Type {
90-
case configv1.AWSPlatformType:
90+
case configv1.AWSPlatformType, configv1.GCPPlatformType, configv1.AzurePlatformType, configv1.OpenStackPlatformType:
9191
set.Insert("169.254.169.254", ic.Networking.MachineCIDR)
92-
default:
93-
return "", fmt.Errorf("unsupported infrastructure provider: %s", infra.Status.PlatformStatus.Type)
9492
}
9593

9694
replicas, err := strconv.Atoi(ic.ControlPlane.Replicas)

0 commit comments

Comments
 (0)