Skip to content

Batch backport of nvmeof service configuration#58477

Merged
Pegonzal merged 3 commits intoceph:squidfrom
afreen23:wip-backport-nvmeof-service
Jul 12, 2024
Merged

Batch backport of nvmeof service configuration#58477
Pegonzal merged 3 commits intoceph:squidfrom
afreen23:wip-backport-nvmeof-service

Conversation

@afreen23
Copy link
Copy Markdown
Contributor

@afreen23 afreen23 commented Jul 9, 2024

Backporting nvmeof service confoguration with its regressions.
Hence, a manual cherrypick of:

https://tracker.ceph.com/issues/66480
https://tracker.ceph.com/issues/66856
https://tracker.ceph.com/issues/66876

original PRs:
#57801
1da0a4f
27a8b2f

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

@afreen23 afreen23 requested a review from a team as a code owner July 9, 2024 09:07
@afreen23 afreen23 requested review from Pegonzal and ivoalmeida and removed request for a team July 9, 2024 09:07
@github-actions github-actions bot added this to the squid milestone Jul 9, 2024
@afreen23
Copy link
Copy Markdown
Contributor Author

afreen23 commented Jul 9, 2024

jenkinst test dashboard

@afreen23 afreen23 changed the title Batch backport of nvmeof service configuration Batch backport of nvmeof service configuration Jul 9, 2024
@afreen23
Copy link
Copy Markdown
Contributor Author

afreen23 commented Jul 9, 2024

jenkinst test dashboard

@afreen23 afreen23 changed the title Batch backport of nvmeof service configuration Batch backport of nvmeof service configuration Jul 9, 2024
@nizamial09 nizamial09 changed the title Batch backport of nvmeof service configuration Batch backport of nvmeof service configuration Jul 9, 2024
@nizamial09
Copy link
Copy Markdown
Member

jenkins test dashboard

@nizamial09
Copy link
Copy Markdown
Member

jenkins test dashboard cephadm

@afreen23 afreen23 changed the title Batch backport of nvmeof service configuration Batch backport of nvmeof service configuration Jul 9, 2024
@afreen23
Copy link
Copy Markdown
Contributor Author

afreen23 commented Jul 9, 2024

jenkins test make check

@afreen23
Copy link
Copy Markdown
Contributor Author

afreen23 commented Jul 9, 2024

jenkins test dashboard cephadm

@afreen23
Copy link
Copy Markdown
Contributor Author

afreen23 commented Jul 9, 2024

jenkins test make check

afreen23 added 3 commits July 10, 2024 13:00
Fixes https://tracker.ceph.com/issues/63686

- creation of Nvme-oF/TCP service
- deletion of Nvme-oF/TCP service
- edit/update Nvme-oF/TCP service
- added unit tests for Nvme-oF/TCP service
- changed Id -> Service Name
- added prefix of service type in service name (similar to <client.> in
  fs access)
- service name and pool are required fields for nvmeof
- placement count now takes default value as mentioned in cephadm
- slight refactors
- prepopulate serviceId for each service type setServiceId()
- in case serviceId is same as servcie type then do not add create service name with<servicetype>.<setrviceid> format

Signed-off-by: Afreen <afreen23.git@gmail.com>
(cherry picked from commit c6cf917)
- service page now uses defaults value for the placement count due to which mds test failing
- in test we pass "1" while "2" which is the default count for mds is already populated, making it 21 and causing unable to create mds service

Fixes: https://tracker.ceph.com/issues/66540
Signed-off-by: Afreen Misbah <afreen23.git@gmail.com>
(cherry picked from commit 5a71822)
(cherry picked from commit 1da0a4f)

Conflicts:
	src/pybind/mgr/dashboard/frontend/cypress/e2e/pools/pools.e2e-spec.ts
	src/pybind/mgr/dashboard/frontend/cypress/e2e/pools/pools.po.ts
Fixes https://tracker.ceph.com/issues/66608

- for services which do not have a count set default count to be null, otherwise the previous selected service's count is used which is wrong
- make count null when label is selected for placement

Signed-off-by: Afreen Misbah <afreen23.git@gmail.com>
(cherry picked from commit 27a8b2f)

Conflicts:
	src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.html
	src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.html
@afreen23 afreen23 force-pushed the wip-backport-nvmeof-service branch from 73d62c8 to 5d2ff4f Compare July 10, 2024 07:33
@afreen23
Copy link
Copy Markdown
Contributor Author

jenkins test dashboard

@afreen23
Copy link
Copy Markdown
Contributor Author

jenkins test make check

@afreen23
Copy link
Copy Markdown
Contributor Author

jenkins test docs

@afreen23
Copy link
Copy Markdown
Contributor Author

jenkins test make check

1 similar comment
@afreen23
Copy link
Copy Markdown
Contributor Author

jenkins test make check

Copy link
Copy Markdown
Contributor

@Pegonzal Pegonzal left a comment

Choose a reason for hiding this comment

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

Thanks @afreen23!

PR looks good to me!

There is just one small issue on the second commit mgr/dashboard: fix service page e2e tests, as there are two different commits being referenced on the cherry-pick

(cherry picked from commit 5a71822)
(cherry picked from commit 1da0a4f)

Could you get that solved?

@afreen23
Copy link
Copy Markdown
Contributor Author

Thanks @afreen23!

PR looks good to me!

There is just one small issue on the second commit mgr/dashboard: fix service page e2e tests, as there are two different commits being referenced on the cherry-pick

(cherry picked from commit 5a71822) (cherry picked from commit 1da0a4f)

Could you get that solved?

Its present in original PR as well. See here 1da0a4f
Thats because @nizamial09 cherrypicked my changes and created a PR

@Pegonzal Pegonzal merged commit 9798a35 into ceph:squid Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants