Skip to content

Commit

Permalink
add back notebook container env var from central proxy config logic s…
Browse files Browse the repository at this point in the history
…o notebook containers have HTTP_PROXY, HTTPS_PROXY, NO_PROXY env vars injected if central cluster proxy config exists

Signed-off-by: Sven Thoms <[email protected]>
  • Loading branch information
shalberd committed Jan 3, 2025
1 parent 0ae60aa commit 78d2210
Showing 1 changed file with 85 additions and 0 deletions.
85 changes: 85 additions & 0 deletions components/odh-notebook-controller/controllers/notebook_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"context"
"encoding/json"
"fmt"
configv1 "github.com/openshift/api/config/v1"
"net/http"
"sort"
"strings"
Expand Down Expand Up @@ -53,6 +54,8 @@ type NotebookWebhook struct {
Namespace string
}

var proxyEnvVars = make(map[string]string, 3)

// InjectReconciliationLock injects the kubeflow notebook controller culling
// stop annotation to explicitly start the notebook pod when the ODH notebook
// controller finishes the reconciliation. Otherwise, a race condition may happen
Expand Down Expand Up @@ -230,6 +233,29 @@ func InjectOAuthProxy(notebook *nbv1.Notebook, oauth OAuthConfig) error {
return nil
}

func (w *NotebookWebhook) ClusterWideProxyIsEnabled() bool {
proxyResourceList := &configv1.ProxyList{}
err := w.Client.List(context.TODO(), proxyResourceList)
if err != nil {
return false
}

for _, proxy := range proxyResourceList.Items {
if proxy.Name == "cluster" {
if proxy.Status.HTTPProxy != "" && proxy.Status.HTTPSProxy != "" &&
proxy.Status.NoProxy != "" {
// Update Proxy Env variables map
proxyEnvVars["HTTP_PROXY"] = proxy.Status.HTTPProxy
proxyEnvVars["HTTPS_PROXY"] = proxy.Status.HTTPSProxy
proxyEnvVars["NO_PROXY"] = proxy.Status.NoProxy
return true
}
}
}
return false

}

// Handle transforms the Notebook objects.
func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) admission.Response {

Expand Down Expand Up @@ -279,6 +305,14 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm
}
}

// If cluster-wide-proxy is enabled add environment variables
if w.ClusterWideProxyIsEnabled() {
err = InjectProxyConfigEnvVars(notebook)
if err != nil {
return admission.Errored(http.StatusInternalServerError, err)
}
}

// RHOAIENG-14552: Running notebook cannot be updated carelessly, or we may end up restarting the pod when
// the webhook runs after e.g. the oauth-proxy image has been updated
mutatedNotebook, needsRestart, err := w.maybeRestartRunningNotebook(ctx, req, notebook)
Expand Down Expand Up @@ -369,6 +403,57 @@ func (w *NotebookWebhook) maybeRestartRunningNotebook(ctx context.Context, req a
return mutatedNotebook, &UpdatesPending{Reason: diff}, nil
}

func InjectProxyConfigEnvVars(notebook *nbv1.Notebook) error {
notebookContainers := &notebook.Spec.Template.Spec.Containers
var imgContainer corev1.Container

// Update Notebook Image container with env variables from central cluster proxy config
for _, container := range *notebookContainers {
// Update notebook image container with env Variables
if container.Name == notebook.Name {
var newVars []corev1.EnvVar
imgContainer = container

for key, val := range proxyEnvVars {
keyExists := false
for _, env := range imgContainer.Env {
if key == env.Name {
keyExists = true
// Update if Proxy spec is updated
if env.Value != val {
env.Value = val
}
}
}
if !keyExists {
newVars = append(newVars, corev1.EnvVar{Name: key, Value: val})
}
}

// Update container only when required env variables are not present
imgContainerExists := false
if len(newVars) != 0 {
imgContainer.Env = append(imgContainer.Env, newVars...)
}

// Update container with Proxy Env Changes
for index, container := range *notebookContainers {
if container.Name == notebook.Name {
(*notebookContainers)[index] = imgContainer
imgContainerExists = true
break
}
}

if !imgContainerExists {
return fmt.Errorf("notebook image container not found %v", notebook.Name)
}
break
}
}
return nil
}

// CheckAndMountCACertBundle checks if the odh-trusted-ca-bundle ConfigMap is present
func CheckAndMountCACertBundle(ctx context.Context, cli client.Client, notebook *nbv1.Notebook, log logr.Logger) error {

Expand Down

0 comments on commit 78d2210

Please sign in to comment.