Skip to content

Commit c835190

Browse files
committed
UPSTREAM: <carry>: Fix a bug ListArtifacts and Getartifacts with older rhoai releases
This bug was introduced after kfp 2.2.0 code rebase Signed-off-by: Ricardo M. Oliveira <[email protected]>
1 parent 028d903 commit c835190

File tree

3 files changed

+105
-6
lines changed

3 files changed

+105
-6
lines changed

backend/src/apiserver/resource/resource_manager.go

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1967,7 +1967,40 @@ func (r *ResourceManager) GetArtifactSessionInfo(ctx context.Context, artifact *
19671967
}
19681968

19691969
// Retrieve Session info
1970-
sessionInfoString := artifactCtx.CustomProperties["store_session_info"].GetStringValue()
1970+
storeSessionInfo, ok_session := artifactCtx.CustomProperties["store_session_info"]
1971+
1972+
var sessionInfoString = ""
1973+
1974+
if ok_session {
1975+
sessionInfoString = storeSessionInfo.GetStringValue()
1976+
} else {
1977+
// bucket_session_info is an old struct that needs to be converted to store_session_info
1978+
bucketSession := &objectstore.S3Params{}
1979+
err1 := json.Unmarshal([]byte(artifactCtx.CustomProperties["bucket_session_info"].GetStringValue()), bucketSession)
1980+
if err1 != nil {
1981+
return nil, "", err1
1982+
}
1983+
sessionInfoParams := &map[string]string{
1984+
"fromEnv": "false",
1985+
"endpoint": bucketSession.Endpoint,
1986+
"region": bucketSession.Region,
1987+
"disableSSL": strconv.FormatBool(bucketSession.DisableSSL),
1988+
"secretName": bucketSession.SecretName,
1989+
"accessKeyKey": bucketSession.AccessKeyKey,
1990+
"secretKeyKey": bucketSession.SecretKeyKey,
1991+
}
1992+
1993+
sessionInfo := &objectstore.SessionInfo{
1994+
Provider: "s3",
1995+
Params: *sessionInfoParams,
1996+
}
1997+
sessionInfoBytes, err2 := json.Marshal(*sessionInfo)
1998+
if err2 != nil {
1999+
return nil, "", err2
2000+
}
2001+
sessionInfoString = string(sessionInfoBytes)
2002+
}
2003+
19712004
if sessionInfoString == "" {
19722005
return nil, "", fmt.Errorf("Unable to retrieve artifact session info via context property.")
19732006
}

backend/src/apiserver/resource/resource_manager_test.go

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,13 @@ import (
1818
"context"
1919
"encoding/json"
2020
"fmt"
21-
"github.com/kubeflow/pipelines/backend/src/v2/objectstore"
2221
"strings"
2322
"testing"
2423
"time"
2524

25+
"github.com/kubeflow/pipelines/backend/src/v2/objectstore"
26+
"github.com/kubeflow/pipelines/third_party/ml-metadata/go/ml_metadata"
27+
2628
"github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
2729
"github.com/argoproj/argo-workflows/v3/util/file"
2830
"github.com/kubeflow/pipelines/backend/src/apiserver/client"
@@ -43,6 +45,14 @@ import (
4345
"k8s.io/apimachinery/pkg/types"
4446
)
4547

48+
func intPtr(i int64) *int64 {
49+
return &i
50+
}
51+
52+
func strPtr(i string) *string {
53+
return &i
54+
}
55+
4656
func initEnvVars() {
4757
viper.Set(common.PodNamespace, "ns1")
4858
}
@@ -4020,6 +4030,35 @@ func TestCreateTask(t *testing.T) {
40204030
assert.Equal(t, expectedTask, storedTask, "The StoredTask return has unexpected value")
40214031
}
40224032

4033+
func TestBackwardsCompatibilityForSessionInfo(t *testing.T) {
4034+
_, manager, _, _, _, _ := initWithExperimentAndPipelineAndRun(t)
4035+
4036+
// First Artifact has assigned a bucket_session_info
4037+
artifact1 := &ml_metadata.Artifact{
4038+
Id: intPtr(0),
4039+
Uri: strPtr("s3://test-bucket/pipeline/some-pipeline-id/task/key0"),
4040+
}
4041+
4042+
config1, _, err := manager.GetArtifactSessionInfo(context.Background(), artifact1)
4043+
4044+
// Assert the results
4045+
assert.NoError(t, err)
4046+
assert.NotNil(t, config1)
4047+
4048+
// Second Artifact has assigned a store_session_info
4049+
artifact2 := &ml_metadata.Artifact{
4050+
Id: intPtr(1),
4051+
Uri: strPtr("s3://test-bucket/pipeline/some-pipeline-id/task/key1"),
4052+
}
4053+
4054+
// Call the function
4055+
config2, _, err := manager.GetArtifactSessionInfo(context.Background(), artifact2)
4056+
4057+
// Assert the results
4058+
assert.NoError(t, err)
4059+
assert.NotNil(t, config2)
4060+
}
4061+
40234062
var v2SpecHelloWorld = `
40244063
components:
40254064
comp-hello-world:

backend/src/v2/metadata/client_fake.go

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,25 +174,52 @@ func (c *FakeClient) createDummyData() {
174174
AccessKeyKey: "testsecretaccesskey",
175175
SecretKeyKey: "testsecretsecretkey",
176176
}
177-
storeSessionInfo, err := json.Marshal(ctx1SessInfo)
177+
bucketSessionInfo, err := json.Marshal(ctx1SessInfo)
178178
if err != nil {
179179
glog.Fatal("failed to marshal fake session info")
180180
}
181+
ctx2SessInfo := map[string]string{
182+
"Region": "test2",
183+
"Endpoint": "test2.endpoint2",
184+
"DisableSSL": "false",
185+
"SecretName": "testsecret2",
186+
"AccessKeyKey": "testsecretaccesskey2",
187+
"SecretKeyKey": "testsecretsecretkey2",
188+
"FromEnv": "false",
189+
}
190+
sessInfo := &objectstore.SessionInfo{
191+
Provider: "s3",
192+
Params: ctx2SessInfo,
193+
}
194+
storeSessionInfo2, err1 := json.Marshal(sessInfo)
195+
if err1 != nil {
196+
glog.Fatal("failed to marshal fake session info")
197+
}
181198

182199
ctx1 := &pb.Context{
183200
Id: intPtr(0),
184201
Name: strPtr("ctx-0"),
185202
Type: strPtr("1"),
203+
CustomProperties: map[string]*pb.Value{
204+
"pipeline_root": stringValue("s3://test-bucket"),
205+
"bucket_session_info": stringValue(string(bucketSessionInfo)),
206+
"namespace": stringValue("test-namespace"),
207+
},
208+
}
209+
ctx2 := &pb.Context{
210+
Id: intPtr(1),
211+
Name: strPtr("ctx-1"),
212+
Type: strPtr("1"),
186213
CustomProperties: map[string]*pb.Value{
187214
"pipeline_root": stringValue("s3://test-bucket"),
188-
"store_session_info": stringValue(string(storeSessionInfo)),
215+
"store_session_info": stringValue(string(storeSessionInfo2)),
189216
"namespace": stringValue("test-namespace"),
190217
},
191218
}
192-
c.contexts = []*pb.Context{ctx1}
219+
c.contexts = []*pb.Context{ctx1, ctx2}
193220
c.artifacts = []*pb.Artifact{art1, art2}
194221
c.artifactIdsToContext = map[int64]*pb.Context{
195222
*art1.Id: ctx1,
196-
*art2.Id: ctx1,
223+
*art2.Id: ctx2,
197224
}
198225
}

0 commit comments

Comments
 (0)