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

Remove RHOSAK #339

Conversation

andrewballantyne
Copy link
Contributor

Copy link
Contributor

@lucferbux lucferbux left a comment

Choose a reason for hiding this comment

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

/lgtm

@andrewballantyne
Copy link
Contributor Author

@bdattoma need a QE approval here -- I saw you made a comment on the RHODS ticket, so I figure I'll tag you here.

@bdattoma
Copy link

@bdattoma need a QE approval here -- I saw you made a comment on the RHODS ticket, so I figure I'll tag you here.

A question: is the rhosak-validation-result automatically created by deploying odh-dashboard/apps-on-prem/rhosak/rhosak-app.yaml?

@andrewballantyne
Copy link
Contributor Author

@bdattoma need a QE approval here -- I saw you made a comment on the RHODS ticket, so I figure I'll tag you here.

A question: is the rhosak-validation-result automatically created by deploying odh-dashboard/apps-on-prem/rhosak/rhosak-app.yaml?

I have no idea -- @lucferbux ?

I'm only the author of this PR because everyone was standing around pointing fingers at each other like the Spiderman meme. I have no knowledge on anything here, I just deleted the resources as they were labeled RHOSAK. Can't deploy RHOSAK if there are no resources to deploy.

@lucferbux
Copy link
Contributor

@bdattoma need a QE approval here -- I saw you made a comment on the RHODS ticket, so I figure I'll tag you here.

A question: is the rhosak-validation-result automatically created by deploying odh-dashboard/apps-on-prem/rhosak/rhosak-app.yaml?

I have no idea -- @lucferbux ?

I'm only the author of this PR because everyone was standing around pointing fingers at each other like the Spiderman meme. I have no knowledge on anything here, I just deleted the resources as they were labeled RHOSAK. Can't deploy RHOSAK if there are no resources to deploy.

No idea either, I don't see that file deployed anywhere and I don't have any context about it, not sure how or where is used.

@bdattoma
Copy link

bdattoma commented May 24, 2023

@bdattoma need a QE approval here -- I saw you made a comment on the RHODS ticket, so I figure I'll tag you here.

A question: is the rhosak-validation-result automatically created by deploying odh-dashboard/apps-on-prem/rhosak/rhosak-app.yaml?

I have no idea -- @lucferbux ?
I'm only the author of this PR because everyone was standing around pointing fingers at each other like the Spiderman meme. I have no knowledge on anything here, I just deleted the resources as they were labeled RHOSAK. Can't deploy RHOSAK if there are no resources to deploy.

No idea either, I don't see that file deployed anywhere and I don't have any context about it, not sure how or where is used.

I did a try creating a custom tile in the dashboard (i.e., OdhApplication object) with the enablement flow. It creates the config map automatically after clicking on "Enable" button in Explore page.
But, when you delete the OdhApplication object, it doesn't delete the CM if the App has been enabled at least once. If we have customers who enabled RHOSAK, imo we could either:

  • detect and delete the CM in the deployer
  • release note to delete the CM manually - don't like much, because user would touch the reserved NS of RHODS

Wdyt? There's probably no harm in having that CM floating in the redhat-ods-applications namespace.. @lucferbux @andrewballantyne

This is the app definition I used, in case you wanna give a try yourselves:

apiVersion: dashboard.opendatahub.io/v1
kind: OdhApplication
metadata:
  labels:
    app: custom-odsci-app
  name: custom-odsci-app
  namespace: redhat-ods-applications
spec:
  enable:
    actionLabel: Enable
    description: Clicking enable will add a card to the Enabled page
    link: 'https://github.com/red-hat-data-services/ods-ci'
    linkPreface: 'Before enabling, be sure you can access:'
    title: Enable Custom ODS-CI App
    validationConfigMap: odsci-validation-result
  img: "image_path_or_svg"
  getStartedLink: 'https://github.com/red-hat-data-services/ods-ci'
  displayName: TEST - Custom ODS-CI Application
  kfdefApplications: []
  support: redhat
  csvName: custom-odsci-app
  provider: ods-ci
  docsLink: 'https://github.com/red-hat-data-services/ods-ci'
  getStartedMarkDown: >-
    # ODS-CI

    ## Your RHODS Tester...Robotic! Develop automated tests for verifying
    functioning of RHODS

    ODS-CI is available on GitHub ### Subscribe to the operator on Marketplace -
    [https://github.com/red-hat-data-services/ods-ci](https://github.com/red-hat-data-services/ods-ci)
  description: >-
    Custom Application is a test item created by ODS-CI to test creation of
    custom doc resources.
  category: Red Hat managed

@andrewballantyne
Copy link
Contributor Author

If there is a configmap created by the effort -- we'd need some build upgrade script to delete it. I'm not in the business of writing scripts to delete resources on a customer cluster.

@bdattoma please reply on the ticket and tag Jeff.

Pretty sure we want these resources gone, the unused configmap should be cleaned up by us in some fashion -- but that's really not for this PR.

@openshift-ci
Copy link

openshift-ci bot commented May 25, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bdattoma, lucferbux

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@andrewballantyne andrewballantyne added the qe-approved The PR has been approved by the QE team label May 25, 2023
@andrewballantyne
Copy link
Contributor Author

Added the label based on Berto approving.

@openshift-merge-robot openshift-merge-robot merged commit a5a21c9 into red-hat-data-services:main May 25, 2023
1 check passed
dimakis pushed a commit to dimakis/odh-deployer that referenced this pull request Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm qe-approved The PR has been approved by the QE team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants