Skip to content

Commit

Permalink
Merge pull request #624 from hbelmiro/RHOAIENG-4252
Browse files Browse the repository at this point in the history
Set ScheduledWorkflow OwnerReference
  • Loading branch information
HumairAK authored Apr 30, 2024
2 parents e914f1d + 99acf3e commit ed6b13c
Show file tree
Hide file tree
Showing 12 changed files with 77 additions and 14 deletions.
2 changes: 2 additions & 0 deletions config/base/params.env
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ MAX_CONCURRENT_RECONCILES=10
DSPO_HEALTHCHECK_DATABASE_CONNECTIONTIMEOUT=15s
DSPO_HEALTHCHECK_OBJECTSTORE_CONNECTIONTIMEOUT=15s
DSPO_REQUEUE_TIME=20s
# DSPO_APISERVER_INCLUDE_OWNERREFERENCE is intended to be used only for tests. It must always be enabled in production
DSPO_APISERVER_INCLUDE_OWNERREFERENCE=true
10 changes: 10 additions & 0 deletions config/internal/apiserver/default/deployment.yaml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ spec:
spec:
containers:
- env:
{{ if .IncludeOwnerReference }}
- name: OWNER_UID
value: "{{.UID}}"
- name: OWNER_NAME
value: "{{.Name}}"
- name: OWNER_API_VERSION
value: "{{.APIVersion}}"
- name: OWNER_KIND
value: "{{.Kind}}"
{{ end }}
- name: POD_NAMESPACE
value: "{{.Namespace}}"
- name: DBCONFIG_USER
Expand Down
38 changes: 24 additions & 14 deletions controllers/config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,20 +80,21 @@ const (

// DSPO Config File Paths
const (
APIServerImagePath = "Images.ApiServer"
APIServerArtifactImagePath = "Images.Artifact"
PersistenceAgentImagePath = "Images.PersistentAgent"
ScheduledWorkflowImagePath = "Images.ScheduledWorkflow"
APIServerCacheImagePath = "Images.Cache"
APIServerMoveResultsImagePath = "Images.MoveResultsImage"
MariaDBImagePath = "Images.MariaDB"
OAuthProxyImagePath = "Images.OAuthProxy"
MlmdEnvoyImagePath = "Images.MlmdEnvoy"
MlmdGRPCImagePath = "Images.MlmdGRPC"
MlmdWriterImagePath = "Images.MlmdWriter"
ObjStoreConnectionTimeoutConfigName = "DSPO.HealthCheck.ObjectStore.ConnectionTimeout"
DBConnectionTimeoutConfigName = "DSPO.HealthCheck.Database.ConnectionTimeout"
RequeueTimeConfigName = "DSPO.RequeueTime"
APIServerImagePath = "Images.ApiServer"
APIServerArtifactImagePath = "Images.Artifact"
PersistenceAgentImagePath = "Images.PersistentAgent"
ScheduledWorkflowImagePath = "Images.ScheduledWorkflow"
APIServerCacheImagePath = "Images.Cache"
APIServerMoveResultsImagePath = "Images.MoveResultsImage"
MariaDBImagePath = "Images.MariaDB"
OAuthProxyImagePath = "Images.OAuthProxy"
MlmdEnvoyImagePath = "Images.MlmdEnvoy"
MlmdGRPCImagePath = "Images.MlmdGRPC"
MlmdWriterImagePath = "Images.MlmdWriter"
ObjStoreConnectionTimeoutConfigName = "DSPO.HealthCheck.ObjectStore.ConnectionTimeout"
DBConnectionTimeoutConfigName = "DSPO.HealthCheck.Database.ConnectionTimeout"
RequeueTimeConfigName = "DSPO.RequeueTime"
ApiServerIncludeOwnerReferenceConfigName = "DSPO.ApiServer.IncludeOwnerReference"
)

// DSPV2-Argo Image Paths
Expand Down Expand Up @@ -172,6 +173,8 @@ const DefaultMaxConcurrentReconciles = 10

const DefaultRequeueTime = time.Second * 20

const DefaultApiServerIncludeOwnerReferenceConfigName = true

func GetConfigRequiredFields() []string {
return requiredFields
}
Expand Down Expand Up @@ -218,6 +221,13 @@ func GetDurationConfigWithDefault(configName string, value time.Duration) time.D
return viper.GetDuration(configName)
}

func GetBoolConfigWithDefault(configName string, value bool) bool {
if !viper.IsSet(configName) {
return value
}
return viper.GetBool(configName)
}

// GetCABundleFileMountPath provides the location in pipeline step-copy-artifact step where the
// ca bundle is mounted for aws cli to connect to s3 store.
// Since pipeline step-copy-artifact step uses aws cli, and there are issues surrounding
Expand Down
17 changes: 17 additions & 0 deletions controllers/dspipeline_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ import (
)

type DSPAParams struct {
IncludeOwnerReference bool
UID types.UID
Name string
APIVersion string
Kind string
Namespace string
Owner mf.Owner
DSPVersion string
Expand Down Expand Up @@ -538,6 +542,17 @@ func (p *DSPAParams) SetupMLMD(dsp *dspa.DataSciencePipelinesApplication, log lo
return nil
}

func (p *DSPAParams) SetupOwner(dsp *dspa.DataSciencePipelinesApplication) {
p.IncludeOwnerReference = config.GetBoolConfigWithDefault(config.ApiServerIncludeOwnerReferenceConfigName, config.DefaultApiServerIncludeOwnerReferenceConfigName)

if p.IncludeOwnerReference {
p.UID = dsp.UID
p.Name = dsp.Name
p.APIVersion = dsp.APIVersion
p.Kind = dsp.Kind
}
}

func setStringDefault(defaultValue string, value *string) {
if *value == "" {
*value = defaultValue
Expand Down Expand Up @@ -801,5 +816,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
return err
}

p.SetupOwner(dsp)

return nil
}
3 changes: 3 additions & 0 deletions controllers/testdata/declarative/case_0/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,6 @@ Images:
MariaDB: mariadb:test0
Minio: minio:test0
OAuthProxy: oauth-proxy:test0
DSPO:
ApiServer:
IncludeOwnerReference: false
3 changes: 3 additions & 0 deletions controllers/testdata/declarative/case_2/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ Images:
MoveResultsImage: busybox:test2
MariaDB: mariadb:test2
OAuthProxy: oauth-proxy:test2
DSPO:
ApiServer:
IncludeOwnerReference: false
3 changes: 3 additions & 0 deletions controllers/testdata/declarative/case_3/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ Images:
MoveResultsImage: busybox:test3
MariaDB: mariadb:test3
OAuthProxy: oauth-proxy:test3
DSPO:
ApiServer:
IncludeOwnerReference: false
3 changes: 3 additions & 0 deletions controllers/testdata/declarative/case_4/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ Images:
MoveResultsImage: this-busybox-image-from-config-should-not-be-used:test4
MariaDB: this-mariadb-image-from-config-should-not-be-used:test4
OAuthProxy: oauth-proxy:test4
DSPO:
ApiServer:
IncludeOwnerReference: false
3 changes: 3 additions & 0 deletions controllers/testdata/declarative/case_5/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,6 @@ Images:
MlmdEnvoy: metadata-envoy:changeme
MlmdGrpc: metadata-grpc:changeme
MlmdWriter: metadata-grpc:changeme
DSPO:
ApiServer:
IncludeOwnerReference: false
3 changes: 3 additions & 0 deletions controllers/testdata/declarative/case_6/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,6 @@ ImagesV2:
ArgoLauncherImage: argolauncherimage:test6
ArgoDriverImage: argodriverimage:test6
MlmdGRPC: quay.io/opendatahub/mlmd-grpc-server:testdsp6
DSPO:
ApiServer:
IncludeOwnerReference: false
3 changes: 3 additions & 0 deletions controllers/testdata/declarative/case_7/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,6 @@ Images:
MariaDB: mariadb:test7
Minio: minio:test7
OAuthProxy: oauth-proxy:test7
DSPO:
ApiServer:
IncludeOwnerReference: false
3 changes: 3 additions & 0 deletions controllers/testdata/declarative/case_8/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ ImagesV2:
ApiServer: api-server:test8
ArgoLauncherImage: argolauncherimage:test8
ArgoDriverImage: argodriverimage:test8
DSPO:
ApiServer:
IncludeOwnerReference: false

0 comments on commit ed6b13c

Please sign in to comment.