Status handling for RouteHealth #399
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
jhadvig
force-pushed
the
route-health
branch
2 times, most recently
from
66ff13d
to
a23ec0b
Compare
jhadvig
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
pkg/api/api.go
Outdated
|
|
||
| const ( | ||
| DefaultIngressCertFilePath = "/var/default-ingress-cert/ca-bundle.crt" | ||
| OAuthEndpointCAFilePath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" |
pkg/console/operator/sync_v400.go
Outdated
| caCertPool = x509.NewCertPool() | ||
| } | ||
|
|
||
| trustedCA, tcaErr := co.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Get(api.TrustedCAConfigMapName, metav1.GetOptions{}) |
There was a problem hiding this comment.
You may want to have a cached client here (if it isn't already).
pkg/console/operator/sync_v400.go
Outdated
|
|
||
| trustedCA, tcaErr := co.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Get(api.TrustedCAConfigMapName, metav1.GetOptions{}) | ||
| if tcaErr != nil { | ||
| klog.V(4).Infof("failed to GET configmap %s in %s (synced from %ss)", api.TrustedCAConfigMapName, api.OpenShiftConsoleNamespace, api.OpenShiftConfigManagedNamespace) |
There was a problem hiding this comment.
Is this true? The trusted-CAs should be retrieved by having a CM be injected by the network-operator rather than being synced.
pkg/console/operator/sync_v400.go
Outdated
| caCertPool.AppendCertsFromPEM([]byte(trustedCABundle)) | ||
|
|
||
| ingressCA, icaErr := co.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Get(api.DefaultIngressCertConfigMapName, metav1.GetOptions{}) | ||
| if icaErr != nil { |
There was a problem hiding this comment.
for _, cmName := range []string{api.TrustedCAConfigMapName, api.DefaultIngressCertConfigMapName} {
cm, err := co.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Get(cmName, metav1.GetOptions{})
if err != nil {
klog.V(4).Infof("failed to GET configmap %s / %s ", api.OpenShiftConsoleNamespace, cmName)
}
caCertPool.AppendCertsFromPEM([]byte(cm.Data["ca.crt"]))
}or you may consider mounting the configmaps
pkg/console/operator/sync_v400.go
Outdated
| } | ||
|
|
||
| func (co *consoleOperator) getCA() (*x509.CertPool, error) { | ||
| caCertPool, _ := x509.SystemCertPool() |
There was a problem hiding this comment.
the trusted-ca bundle should contain all the certificates necessary to communicate with the outer world so you may omit this or mount it to the operator
pkg/console/operator/sync_v400.go
Outdated
| return nil, tcaErr | ||
| } | ||
| trustedCABundle := trustedCA.Data[api.TrustedCABundleKey] | ||
| caCertPool.AppendCertsFromPEM([]byte(trustedCABundle)) |
There was a problem hiding this comment.
maybe add a debug log if AppendCertsFromPEM returns false
|
@stlaz comments addressed. Thanks ! |
pkg/console/operator/sync_v400.go
Outdated
| @@ -61,6 +65,7 @@ func (co *consoleOperator) sync_v400(updatedOperatorConfig *operatorv1.Console, | |||
| if rtErr != nil { | |||
| return rtErr | |||
| } | |||
| co.CheckRouteHealth(updatedOperatorConfig, rt) | |||
There was a problem hiding this comment.
I'm inclined to move this to the route controller here:
After the sync.
|
@benjaminapetersen comments addressed! PTAL |
|
/retest |
1 similar comment
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: benjaminapetersen, jhadvig The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
Rework of #334
/assign @benjaminapetersen