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: model upload failed with model group level access control #677

Merged

Conversation

wanglam
Copy link
Collaborator

@wanglam wanglam commented May 25, 2023

Description

The ml-commons 2.8 will bring model group level access control feature. The new feature will break old model upload process. This PR enable model register upload by URL and create model group before model upload. Then model can be uploaded and deployed.
This PR should be backport to 2.x branch and already passed in the local environment.

Test video: https://github.com/opensearch-project/opensearch-dashboards-functional-test/assets/4034161/6044acbe-9be6-48fb-94df-2d3551cd506c

Test screenshot: image

Issues Resolved

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Lin Wang <wonglam@amazon.com>
@peterzhuamazon
Copy link
Member

peterzhuamazon commented May 25, 2023

I will fix the test failure which is most likely due to startup, not tar, not test cases.

@peterzhuamazon peterzhuamazon added the bug Something isn't working label May 25, 2023
@peterzhuamazon
Copy link
Member

peterzhuamazon commented May 25, 2023

Hi @wanglam seems like there are still some errors even in local test:

sec enabled log
$ ./integtest.sh -s true -t mlCommonsDashboards -v 3.0.0
npm WARN old lockfile
npm WARN old lockfile The package-lock.json file was created with an old version of npm,
npm WARN old lockfile so supplemental metadata must be fetched from the registry.
npm WARN old lockfile
npm WARN old lockfile This is a one-time fix-up, please be patient...
npm WARN old lockfile

> opensearch-dashboards-functional-test@3.0.0 postinstall
> husky install

husky - Git hooks installed

added 331 packages, and audited 332 packages in 7s

76 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
npm notice
npm notice New major version of npm available! 8.19.3 -> 9.6.7
npm notice Changelog: https://github.com/npm/cli/releases/tag/v9.6.7
npm notice Run npm install -g npm@9.6.7 to update!
npm notice
Test Files List:
cypress/integration/plugins/ml-commons-dashboards/overview_spec.js
run security enabled tests
yarn run v1.22.19
$ env TZ=America/Los_Angeles NO_COLOR=1 cypress run --headless --env SECURITY_ENABLED=true,openSearchUrl=https://localhost:9200 --browser chromium --spec 'cypress/integration/plugins/ml-commons-dashboards/*'
[18114:0525/120615.222530:ERROR:gpu_init.cc(453)] Passthrough is not supported, GL is swiftshader, ANGLE is
Couldn't determine Mocha version

====================================================================================================

  (Run Starting)

  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ Cypress:        9.5.4                                                                          │
  │ Browser:        Chromium 113 (headless)                                                        │
  │ Node Version:   v16.19.1 (/local/home/zhujiaxi/.nvm/versions/node/v16.19.1/bin/node)           │
  │ Specs:          1 found (plugins/ml-commons-dashboards/overview_spec.js)                       │
  │ Searched:       cypress/integration/plugins/ml-commons-dashboards/*                            │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘


────────────────────────────────────────────────────────────────────────────────────────────────────

  Running:  plugins/ml-commons-dashboards/overview_spec.js                                  (1 of 1)
Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating
Couldn't determine Mocha version


  MLC Overview page
    1) "before all" hook for "should return to monitoring page when visit root"


  0 passing (271ms)
  1 failing

  1) MLC Overview page
       "before all" hook for "should return to monitoring page when visit root":
     CypressError: `cy.request()` failed on:

https://localhost:9200/_cluster/settings

The response we received from your web server was:

  > 400: Bad Request

This was considered a failure because the status code was not `2xx` or `3xx`.

If you do not want status codes to cause failures pass the option: `failOnStatusCode: false`

-----------------------------------------------------------

The request we sent was:

Method: PUT
URL: https://localhost:9200/_cluster/settings
Headers: {
  "Connection": "keep-alive",
  "user-agent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/113.0.5672.63 Safari/537.36",
  "accept": "*/*",
  "authorization": "Basic YWRtaW46YWRtaW4=",
  "accept-encoding": "gzip, deflate",
  "content-type": "application/json",
  "content-length": 62
}
Body: {"transient":{"plugins.ml_commons.only_run_on_ml_node":false}}

-----------------------------------------------------------

The response we got was:

Status: 400 - Bad Request
Headers: {
  "content-type": "application/json; charset=UTF-8",
  "content-length": "269"
}
Body: {
  "error": {
    "root_cause": [
      {
        "type": "settings_exception",
        "reason": "transient setting [plugins.ml_commons.only_run_on_ml_node], not recognized"
      }
    ],
    "type": "settings_exception",
    "reason": "transient setting [plugins.ml_commons.only_run_on_ml_node], not recognized"
  },
  "status": 400
}


https://on.cypress.io/request

Because this error occurred during a `before all` hook we are skipping the remaining tests in the current suite: `MLC Overview page`
      at http://localhost:5601/__cypress/runner/cypress_runner.js:160667:80
      at tryCatcher (http://localhost:5601/__cypress/runner/cypress_runner.js:13022:23)
      at Promise._settlePromiseFromHandler (http://localhost:5601/__cypress/runner/cypress_runner.js:10957:31)
      at Promise._settlePromise (http://localhost:5601/__cypress/runner/cypress_runner.js:11014:18)
      at Promise._settlePromise0 (http://localhost:5601/__cypress/runner/cypress_runner.js:11059:10)
      at Promise._settlePromises (http://localhost:5601/__cypress/runner/cypress_runner.js:11139:18)
      at _drainQueueStep (http://localhost:5601/__cypress/runner/cypress_runner.js:7729:12)
      at _drainQueue (http://localhost:5601/__cypress/runner/cypress_runner.js:7722:9)
      at ../../node_modules/bluebird/js/release/async.js.Async._drainQueues (http://localhost:5601/__cypress/runner/cypress_runner.js:7738:5)
      at Async.drainQueues (http://localhost:5601/__cypress/runner/cypress_runner.js:7608:14)
  From Your Spec Code:
      at Context.eval (http://localhost:5601/__cypress/tests?p=cypress/support/index.js:2375:6)




  (Results)

  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ Tests:        4                                                                                │
  │ Passing:      0                                                                                │
  │ Failing:      1                                                                                │
  │ Pending:      0                                                                                │
  │ Skipped:      3                                                                                │
  │ Screenshots:  1                                                                                │
  │ Video:        true                                                                             │
  │ Duration:     0 seconds                                                                        │
  │ Spec Ran:     plugins/ml-commons-dashboards/overview_spec.js                                   │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘


  (Screenshots)

  -  /local/home/zhujiaxi/2.8.0/opensearch-dashboards-functional-test/cypress/screens     (1280x720)
     hots/plugins/ml-commons-dashboards/overview_spec.js/MLC Overview page -- should
     return to monitoring page when visit root -- before all hook (failed).png


  (Video)

  -  Started processing:  Compressing to 32 CRF
  -  Finished processing: /local/home/zhujiaxi/2.8.0/opensearch-dashboards-functional    (0 seconds)
                          -test/cypress/videos/plugins/ml-commons-dashboards/overview
                          _spec.js.mp4


====================================================================================================

  (Run Finished)


       Spec                                              Tests  Passing  Failing  Pending  Skipped
  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ ✖  plugins/ml-commons-dashboards/overv      271ms        4        -        1        -        3 │
  │    iew_spec.js                                                                                 │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘
    ✖  1 of 1 failed (100%)                     271ms        4        -        1        -        3

@peterzhuamazon
Copy link
Member

overview_spec.js.mp4

@peterzhuamazon
Copy link
Member

Offline discussion with @wanglam that latest ML Commons Dash plugin must be installed on 2.8.0 in order to test this.
Will approve as he provided local test result.

Thanks.

@kavilla kavilla merged commit b159ff0 into opensearch-project:main May 26, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 26, 2023
Signed-off-by: Lin Wang <wonglam@amazon.com>
(cherry picked from commit b159ff0)
peterzhuamazon pushed a commit that referenced this pull request May 26, 2023
…#684)

Signed-off-by: Lin Wang <wonglam@amazon.com>
(cherry picked from commit b159ff0)

Co-authored-by: Lin Wang <wonglam@amazon.com>
leanneeliatra pushed a commit to leanneeliatra/opensearch-dashboards-functional-test-fork that referenced this pull request Sep 15, 2023
…earch-project#677) (opensearch-project#684)

Signed-off-by: Lin Wang <wonglam@amazon.com>
(cherry picked from commit b159ff0)

Co-authored-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: leanne.laceybyrne@eliatra.com <leanne.laceybyrne@eliatra.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

3 participants