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

fix: cleanup runtime default properties on first reconcile, fixes RHOAIENG-15033 #158

Merged

Conversation

dhirajsb
Copy link
Contributor

@dhirajsb dhirajsb commented Nov 5, 2024

Description

Cleanup runtime default properties on first reconcile
Fixes RHOAIENG-15033

How Has This Been Tested?

Tested locally by using an MR with default runtime properties and verified that it gets cleaned up to remove them.

Create test MR using secure-db mysql-tls sample, then delete the test MR and wait for it to be completely removed.

Recreate the MR with default runtime properties using the command:

# deletes old MR just to be safe
oc delete mr -n odh-model-registries modelregistry-sample
# create test MR with default values
oc apply -f - <<EOF
apiVersion: modelregistry.opendatahub.io/v1alpha1
kind: ModelRegistry
metadata:
  labels:
    app.kubernetes.io/created-by: model-registry-operator
    app.kubernetes.io/instance: modelregistry-sample
    app.kubernetes.io/managed-by: kustomize
    app.kubernetes.io/name: modelregistry
    app.kubernetes.io/part-of: model-registry-operator
  name: modelregistry-sample
  namespace: odh-model-registries
spec:
  grpc:
    image: quay.io/opendatahub/mlmd-grpc-server:latest
    port: 9090
    resources:
      limits:
        cpu: 100m
        memory: 256Mi
      requests:
        cpu: 100m
        memory: 256Mi
  istio:
    authProvider: opendatahub-auth-provider
    authConfigLabels:
      security.opendatahub.io/authorization-group: default
    gateway:
      grpc:
        gatewayRoute: enabled
        port: 443
        tls:
          credentialName: default-modelregistry-cert
          mode: SIMPLE
      istioIngress: ingressgateway
      rest:
        gatewayRoute: enabled
        port: 443
        tls:
          credentialName: default-modelregistry-cert
          mode: SIMPLE
    tlsMode: ISTIO_MUTUAL
  mysql:
    database: model_registry
    host: model-registry-db
    passwordSecret:
      key: database-password
      name: model-registry-db
    port: 3306
    skipDBCreation: false
    sslRootCertificateConfigMap:
      key: ca.crt
      name: model-registry-db-credential
    username: mlmduser
  rest:
    image: quay.io/opendatahub/model-registry:latest
    port: 8080
    serviceRoute: disabled
    resources:
      limits:
        cpu: 100m
        memory: 256Mi
      requests:
        cpu: 100m
        memory: 256Mi
EOF

After reconciliation with this fix, the default runtime property values including images should be removed from the above MR.

Merge criteria:

  • The commits and have meaningful messages; the author will squash them after approval or will ask to merge with squash.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@dhirajsb dhirajsb requested a review from a team November 5, 2024 06:35
@dhirajsb dhirajsb force-pushed the feat/cleanup-runtime-defaults branch from 93e3403 to c3147c7 Compare November 6, 2024 20:01
@dhirajsb dhirajsb requested a review from Al-Pragliola November 6, 2024 21:54
Al-Pragliola
Al-Pragliola previously approved these changes Nov 7, 2024
Copy link
Contributor

@Al-Pragliola Al-Pragliola left a comment

Choose a reason for hiding this comment

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

lgtm

@Al-Pragliola Al-Pragliola self-requested a review November 7, 2024 15:32
@dhirajsb dhirajsb force-pushed the feat/cleanup-runtime-defaults branch from c3147c7 to 8fcfb7e Compare November 7, 2024 23:35
@dhirajsb dhirajsb force-pushed the feat/cleanup-runtime-defaults branch 2 times, most recently from 557bc38 to 9ba4491 Compare November 8, 2024 01:35
…ause mutatingwebhook is only called on create or update

Signed-off-by: Dhiraj Bokde <[email protected]>
@dhirajsb dhirajsb force-pushed the feat/cleanup-runtime-defaults branch from 9ba4491 to 74e4fa4 Compare November 8, 2024 01:35
@Al-Pragliola Al-Pragliola merged commit 4dc22a7 into opendatahub-io:main Nov 8, 2024
2 checks passed
@dhirajsb dhirajsb deleted the feat/cleanup-runtime-defaults branch November 19, 2024 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants