Add a new field "TensorflowImage" to KFP viewer CRD file template. (#…

…2544)

* Without version bump

* fix the delete caller

* return after delete

* reconciler removes old viewer crd file that misses image specification

* add frontend comment

* remove accidental changes that are irrelevant

* Revise log message

* Add error handling

* add test

* tensorflow image check only applies to viewer tensorboard type and thus put it after the type check.

* Use of default image instead of validation

backend/src/crd/controller/viewer/reconciler/reconciler.go

@@ -42,6 +42,8 @@ import (
 
 const viewerTargetPort = 6006
 
+const defaultTensorflowImage = "tensorflow/tensorflow:1.13.2"
+
 // Reconciler implements reconcile.Reconciler for the Viewer CRD.
 type Reconciler struct {
 	client.Client
@@ -90,6 +92,10 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 		return reconcile.Result{}, nil
 	}
 
+	if len(view.Spec.TensorboardSpec.TensorflowImage) == 0 {
+		view.Spec.TensorboardSpec.TensorflowImage = defaultTensorflowImage
+	}
+
 	// Check and maybe delete the oldest viewer before creating the next one.
 	if err := r.maybeDeleteOldestViewer(view.Spec.Type, view.Namespace); err != nil {
 		// Couldn't delete. Requeue.
@@ -165,7 +171,7 @@ func setPodSpecForTensorboard(view *viewerV1beta1.Viewer, s *corev1.PodSpec) {
 
 	c := &s.Containers[0]
 	c.Name = view.Name + "-pod"
-	c.Image = "tensorflow/tensorflow:1.13.2"
+	c.Image = view.Spec.TensorboardSpec.TensorflowImage
 	c.Args = []string{
 		"tensorboard",
 		fmt.Sprintf("--logdir=%s", view.Spec.TensorboardSpec.LogDir),

backend/src/crd/controller/viewer/reconciler/reconciler_test.go

@@ -38,6 +38,8 @@ import (
 
 var viewer *Reconciler
 
+const tensorflowImage = "tensorflow/tensorflow:dummy"
+
 func TestMain(m *testing.M) {
 	viewerV1beta1.AddToScheme(scheme.Scheme)
 	os.Exit(m.Run())
@@ -128,7 +130,8 @@ func TestReconcile_EachViewerCreatesADeployment(t *testing.T) {
 		Spec: viewerV1beta1.ViewerSpec{
 			Type: viewerV1beta1.ViewerTypeTensorboard,
 			TensorboardSpec: viewerV1beta1.TensorboardSpec{
-				LogDir: "gs://tensorboard/logdir",
+				LogDir:          "gs://tensorboard/logdir",
+				TensorflowImage: tensorflowImage,
 			},
 		},
 	}
@@ -174,7 +177,7 @@ func TestReconcile_EachViewerCreatesADeployment(t *testing.T) {
 				Spec: corev1.PodSpec{
 					Containers: []corev1.Container{{
 						Name:  "viewer-123-pod",
-						Image: "tensorflow/tensorflow:1.13.2",
+						Image: tensorflowImage,
 						Args: []string{
 							"tensorboard",
 							"--logdir=gs://tensorboard/logdir",
@@ -200,7 +203,8 @@ func TestReconcile_ViewerUsesSpecifiedVolumeMountsForDeployment(t *testing.T) {
 		Spec: viewerV1beta1.ViewerSpec{
 			Type: viewerV1beta1.ViewerTypeTensorboard,
 			TensorboardSpec: viewerV1beta1.TensorboardSpec{
-				LogDir: "gs://tensorboard/logdir",
+				LogDir:          "gs://tensorboard/logdir",
+				TensorflowImage: tensorflowImage,
 			},
 			PodTemplateSpec: corev1.PodTemplateSpec{
 				Spec: corev1.PodSpec{
@@ -271,7 +275,7 @@ func TestReconcile_ViewerUsesSpecifiedVolumeMountsForDeployment(t *testing.T) {
 				Spec: corev1.PodSpec{
 					Containers: []corev1.Container{{
 						Name:  "viewer-123-pod",
-						Image: "tensorflow/tensorflow:1.13.2",
+						Image: tensorflowImage,
 						Args: []string{
 							"tensorboard",
 							"--logdir=gs://tensorboard/logdir",
@@ -311,7 +315,8 @@ func TestReconcile_EachViewerCreatesAService(t *testing.T) {
 		Spec: viewerV1beta1.ViewerSpec{
 			Type: viewerV1beta1.ViewerTypeTensorboard,
 			TensorboardSpec: viewerV1beta1.TensorboardSpec{
-				LogDir: "gs://tensorboard/logdir",
+				LogDir:          "gs://tensorboard/logdir",
+				TensorflowImage: tensorflowImage,
 			},
 		},
 	}
@@ -381,7 +386,8 @@ func TestReconcile_UnknownViewerTypesAreIgnored(t *testing.T) {
 		Spec: viewerV1beta1.ViewerSpec{
 			Type: "unknownType",
 			TensorboardSpec: viewerV1beta1.TensorboardSpec{
-				LogDir: "gs://tensorboard/logdir",
+				LogDir:          "gs://tensorboard/logdir",
+				TensorflowImage: tensorflowImage,
 			},
 		},
 	}
@@ -451,7 +457,8 @@ func makeViewer(id int) (*types.NamespacedName, *viewerV1beta1.Viewer) {
 		Spec: viewerV1beta1.ViewerSpec{
 			Type: viewerV1beta1.ViewerTypeTensorboard,
 			TensorboardSpec: viewerV1beta1.TensorboardSpec{
-				LogDir: "gs://tensorboard/logdir",
+				LogDir:          "gs://tensorboard/logdir",
+				TensorflowImage: tensorflowImage,
 			},
 		},
 	}

backend/src/crd/pkg/apis/viewer/v1beta1/types.go

@@ -59,7 +59,8 @@ const (
 type TensorboardSpec struct {
 	// LogDir is the location of the log directory to be read by tensorboard, i.e.,
 	// ---log_dir.
-	LogDir string `json:"logDir"`
+	LogDir          string `json:"logDir"`
+	TensorflowImage string `json:"tensorflowImage"`
 }
 
 // ViewerSpec is the spec for a Viewer resource.

frontend/server/k8s-helper.ts

@@ -100,6 +100,8 @@ export async function newTensorboardInstance(logdir: string, podTemplateSpec: Ob
       type: 'tensorboard',
       tensorboardSpec: {
         logDir: logdir,
+        // TODO(jingzhang36): tensorflow image version read from input textbox.
+        tensorflowImage: 'tensorflow/tensorflow:1.13.2',
       },
       podTemplateSpec
     }
@@ -175,16 +177,16 @@ export async function getArgoWorkflow(workflowName: string): Promise<IPartialArg
 
   const res = await k8sV1CustomObjectClient.getNamespacedCustomObject(
     workflowGroup, workflowVersion, namespace, workflowPlural, workflowName)
-  
+
   if (res.response.statusCode >= 400) {
     throw new Error(`Unable to query workflow:${workflowName}: Access denied.`);
   }
   return res.body;
 }
-  
+
 /**
  * Retrieves k8s secret by key and decode from base64.
- * @param name name of the secret 
+ * @param name name of the secret
  * @param key key in the secret
  */
 export async function getK8sSecret(name: string, key: string) {
@@ -195,5 +197,5 @@ export async function getK8sSecret(name: string, key: string) {
   const k8sSecret = await k8sV1Client.readNamespacedSecret(name, namespace);
   const secretb64 = k8sSecret.body.data[key];
   const buff = new Buffer(secretb64, 'base64');
-  return buff.toString('ascii');  
+  return buff.toString('ascii');
 }

manifests/kustomize/env/dev/kustomization.yaml

@@ -13,4 +13,4 @@ images:
   - name: mysql
     newTag: "5.6"
   - name: minio/minio
-    newTag: RELEASE.2018-02-09T22-40-05Z
+    newTag: RELEASE.2018-02-09T22-40-05Z