Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update: add dependent resources #68

Merged
merged 8 commits into from
Oct 19, 2024

Conversation

zdtsw
Copy link
Collaborator

@zdtsw zdtsw commented Oct 12, 2024

depend on #67
also fix comments from ^

@zdtsw zdtsw changed the title update: add depedent resources update: add dependent resources Oct 14, 2024
- add missing resoruce in dps
- chore fix from  naming in ModelReg

Signed-off-by: Wen Zhou <[email protected]>
@zdtsw zdtsw requested review from dhirajsb and removed request for rareddy and anishasthana October 15, 2024 14:04
README.md Outdated Show resolved Hide resolved
collection-scripts/gather Outdated Show resolved Hide resolved
zdtsw and others added 2 commits October 15, 2024 17:26
Co-authored-by: Dhiraj Bokde <[email protected]>
Copy link
Contributor

@dhirajsb dhirajsb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add model registry servicemesh resources.

collection-scripts/gather_mr Outdated Show resolved Hide resolved
@dhirajsb
Copy link
Contributor

@zdtsw MR operator also creates Roles, RoleBindings, and Groups specific to model registry. But, I assume that gathering them will potentially expose user PIA. I don't see other components gathering any security related information either.

@zdtsw
Copy link
Collaborator Author

zdtsw commented Oct 15, 2024

@zdtsw MR operator also creates Roles, RoleBindings, and Groups specific to model registry. But, I assume that gathering them will potentially expose user PIA. I don't see other components gathering any security related information either.

if we need such for specific cases, can ask user to provide them if they are ok to share

@zdtsw
Copy link
Collaborator Author

zdtsw commented Oct 16, 2024

@zdtsw zdtsw requested a review from dhirajsb October 17, 2024 12:53
# for DSP
resources=("datasciencepipelinesapplications")
# depedent resources from kubeflow
resources+=("scheduledworkflows" "viewers")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need viewers

# depedent resources from kubeflow
resources+=("scheduledworkflows" "viewers")
# depedent resources from k8s
resources+=("applications")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure where this is referenced from, but I don't think we need this

@zdtsw zdtsw requested a review from HumairAK October 17, 2024 13:01
@@ -2,6 +2,12 @@
# shellcheck disable=SC1091
source common.sh
resources=("inferenceservices" "inferencegraphs" "trainedmodels" "servingruntimes" "clusterstoragecontainers" "predictors")
# Dependent resources ossm
resources+=("smcp" "smm" "smmr" "gateways" "authorizationpolicies" "envoyfilters")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The virtualservice is missing. Additionally, it is not possible to gather the secret, correct? This is because we have TLS secrets in place.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we do not collect secret by default, i wil add virtualservices in the list

Copy link

@israel-hdez israel-hdez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with the Serving part.

Copy link
Contributor

@dhirajsb dhirajsb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@zdtsw zdtsw merged commit e6e42f9 into red-hat-data-services:main Oct 19, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants