From 9957030fc8568936fb8561b47ed09e95a602d7de Mon Sep 17 00:00:00 2001 From: mucahit-kantepe Date: Wed, 26 Apr 2023 14:39:58 -0700 Subject: [PATCH 1/5] Fix double http in the Spark Driver UI Link Signed-off-by: mucahit-kantepe --- go/tasks/plugins/k8s/spark/spark.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/go/tasks/plugins/k8s/spark/spark.go b/go/tasks/plugins/k8s/spark/spark.go index 66600709a..64ea664fc 100755 --- a/go/tasks/plugins/k8s/spark/spark.go +++ b/go/tasks/plugins/k8s/spark/spark.go @@ -407,8 +407,9 @@ func getEventInfoForSpark(sj *sparkOp.SparkApplication) (*pluginsCore.TaskInfo, }) } } else if sj.Status.AppState.State == sparkOp.RunningState && sj.Status.DriverInfo.WebUIIngressAddress != "" { - // Append https as the operator doesn't currently. - customInfoMap[sparkDriverUI] = fmt.Sprintf("https://%s", sj.Status.DriverInfo.WebUIIngressAddress) + // Older versions of spark-operator does not have http:// but newer versions have. + customInfoMap[sparkDriverUI] = strings.TrimPrefix(sj.Status.DriverInfo.WebUIIngressAddress, "http://") + customInfoMap[sparkDriverUI] = fmt.Sprintf("https://%s", customInfoMap[sparkDriverUI]) // Custom doesn't work unless the UI has a custom plugin to parse this, hence add to Logs as well. taskLogs = append(taskLogs, &core.TaskLog{ Uri: customInfoMap[sparkDriverUI], From 796e2450288e329decf0cba37f9259c5cd60ee00 Mon Sep 17 00:00:00 2001 From: mucahit-kantepe Date: Wed, 26 Apr 2023 14:52:45 -0700 Subject: [PATCH 2/5] fix test Signed-off-by: mucahit-kantepe --- go/tasks/plugins/k8s/spark/spark_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/go/tasks/plugins/k8s/spark/spark_test.go b/go/tasks/plugins/k8s/spark/spark_test.go index 6b6ed18d8..78a1f302c 100755 --- a/go/tasks/plugins/k8s/spark/spark_test.go +++ b/go/tasks/plugins/k8s/spark/spark_test.go @@ -2,7 +2,6 @@ package spark import ( "context" - "fmt" "os" "strconv" "testing" @@ -34,7 +33,7 @@ import ( const sparkMainClass = "MainClass" const sparkApplicationFile = "local:///spark_app.py" const testImage = "image://" -const sparkUIAddress = "spark-ui.flyte" +const sparkUIAddress = "http://spark-ui.flyte" var ( dummySparkConf = map[string]string{ @@ -91,7 +90,7 @@ func TestGetEventInfo(t *testing.T) { info, err := getEventInfoForSpark(dummySparkApplication(sj.RunningState)) assert.NoError(t, err) assert.Len(t, info.Logs, 6) - assert.Equal(t, fmt.Sprintf("https://%s", sparkUIAddress), info.CustomInfo.Fields[sparkDriverUI].GetStringValue()) + assert.Equal(t, "https://spark-ui.flyte", info.CustomInfo.Fields[sparkDriverUI].GetStringValue()) generatedLinks := make([]string, 0, len(info.Logs)) for _, l := range info.Logs { generatedLinks = append(generatedLinks, l.Uri) From a069bba9f33bc2e07b815dd95d55c36967ac2c52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=BCcahit=20Kantepe?= Date: Wed, 26 Apr 2023 21:28:21 -0700 Subject: [PATCH 3/5] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ketan Umare <16888709+kumare3@users.noreply.github.com> Signed-off-by: Mücahit Kantepe --- go/tasks/plugins/k8s/spark/spark.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/tasks/plugins/k8s/spark/spark.go b/go/tasks/plugins/k8s/spark/spark.go index 64ea664fc..8e2ee5ee9 100755 --- a/go/tasks/plugins/k8s/spark/spark.go +++ b/go/tasks/plugins/k8s/spark/spark.go @@ -408,8 +408,8 @@ func getEventInfoForSpark(sj *sparkOp.SparkApplication) (*pluginsCore.TaskInfo, } } else if sj.Status.AppState.State == sparkOp.RunningState && sj.Status.DriverInfo.WebUIIngressAddress != "" { // Older versions of spark-operator does not have http:// but newer versions have. - customInfoMap[sparkDriverUI] = strings.TrimPrefix(sj.Status.DriverInfo.WebUIIngressAddress, "http://") - customInfoMap[sparkDriverUI] = fmt.Sprintf("https://%s", customInfoMap[sparkDriverUI]) + uri := strings.TrimPrefix(sj.Status.DriverInfo.WebUIIngressAddress, "http://") + customInfoMap[sparkDriverUI] = fmt.Sprintf("https://%s", uri) // Custom doesn't work unless the UI has a custom plugin to parse this, hence add to Logs as well. taskLogs = append(taskLogs, &core.TaskLog{ Uri: customInfoMap[sparkDriverUI], From 6453fecab7701edcef83b63b95dba7e75b34f7df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=BCcahit=20Kantepe?= Date: Wed, 26 Apr 2023 21:29:59 -0700 Subject: [PATCH 4/5] Update spark.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Mücahit Kantepe --- go/tasks/plugins/k8s/spark/spark.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/tasks/plugins/k8s/spark/spark.go b/go/tasks/plugins/k8s/spark/spark.go index 8e2ee5ee9..f499179b2 100755 --- a/go/tasks/plugins/k8s/spark/spark.go +++ b/go/tasks/plugins/k8s/spark/spark.go @@ -407,7 +407,7 @@ func getEventInfoForSpark(sj *sparkOp.SparkApplication) (*pluginsCore.TaskInfo, }) } } else if sj.Status.AppState.State == sparkOp.RunningState && sj.Status.DriverInfo.WebUIIngressAddress != "" { - // Older versions of spark-operator does not have http:// but newer versions have. + // Older versions of spark-operator does not append http:// but newer versions do. uri := strings.TrimPrefix(sj.Status.DriverInfo.WebUIIngressAddress, "http://") customInfoMap[sparkDriverUI] = fmt.Sprintf("https://%s", uri) // Custom doesn't work unless the UI has a custom plugin to parse this, hence add to Logs as well. From 9fb2b0d65acf21b1c7c3a8deeace06da02350426 Mon Sep 17 00:00:00 2001 From: mucahit-kantepe Date: Fri, 28 Apr 2023 17:01:37 -0700 Subject: [PATCH 5/5] fmt Signed-off-by: mucahit-kantepe --- go/tasks/plugins/k8s/spark/spark.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/tasks/plugins/k8s/spark/spark.go b/go/tasks/plugins/k8s/spark/spark.go index f499179b2..a4e27bad4 100755 --- a/go/tasks/plugins/k8s/spark/spark.go +++ b/go/tasks/plugins/k8s/spark/spark.go @@ -407,7 +407,7 @@ func getEventInfoForSpark(sj *sparkOp.SparkApplication) (*pluginsCore.TaskInfo, }) } } else if sj.Status.AppState.State == sparkOp.RunningState && sj.Status.DriverInfo.WebUIIngressAddress != "" { - // Older versions of spark-operator does not append http:// but newer versions do. + // Older versions of spark-operator does not append http:// but newer versions do. uri := strings.TrimPrefix(sj.Status.DriverInfo.WebUIIngressAddress, "http://") customInfoMap[sparkDriverUI] = fmt.Sprintf("https://%s", uri) // Custom doesn't work unless the UI has a custom plugin to parse this, hence add to Logs as well.