mgr/dashboard: make application field for pool creation mandatory #51566

avanthakkar · 2023-05-18T12:15:30Z

Fixes: https://tracker.ceph.com/issues/61238

Before

After:

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

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)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

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

pereman2

Didn't work. It created the pool after saying it was required.

screen-capture.8.webm

avanthakkar · 2023-05-18T12:32:57Z

Didn't work. It created the pool after saying it was required.
screen-capture.8.webm

Just fixed the submit if no application selected. PTAL!

Pegonzal

Just a NIT

The: This field is required! text only shows if all other fields have been filled

avanthakkar · 2023-05-19T08:17:37Z

jenkins test make check

nizamial09

Probably it should be better to add a helper here saying that a pool cannot be used without associating with an application before

src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-data.ts

avanthakkar · 2023-05-19T09:41:57Z

Probably it should be better to add a helper here saying that a pool cannot be used without associating with an application before

And added a helper for application fields.

avanthakkar · 2023-05-19T12:11:30Z

jenkins test make check

avanthakkar · 2023-05-19T12:12:06Z

jenkins test dashboard

nizamial09 · 2023-05-22T07:09:02Z

make-check failure

● PoolFormComponent › submit - create › erasure coded pool › minimum requirements without ECP to create ec pool

    expect(spy).toHaveBeenCalledWith(...expected)

    Expected: {"pg_autoscale_mode": "off", "pg_num": 4, "pool": "minECPool", "pool_type": "erasure"}

    Number of calls: 0

      82 |     spyOn(taskWrapper, 'wrapTaskAroundCall').and.callThrough();
      83 |     component.submit();
    > 84 |     expect(poolService[poolServiceMethod]).toHaveBeenCalledWith(pool);
         |                                            ^
      85 |     expect(taskWrapper.wrapTaskAroundCall).toHaveBeenCalledWith({
      86 |       task: {
      87 |         name: taskName,

      at expectValidSubmit (src/app/ceph/pool/pool-form/pool-form.component.spec.ts:84:44)
      at src/app/ceph/pool/pool-form/pool-form.component.spec.ts:1108:9
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:409:30)
      at ProxyZoneSpec.Object.<anonymous>.ProxyZoneSpec.onInvoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:3830:43)
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:408:56)
      at Zone.Object.<anonymous>.Zone.run (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:169:47)
      at Object.wrappedFunc (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:4330:34)

  ● PoolFormComponent › submit - create › erasure coded pool › creates ec pool with erasure coded profile

    expect(spy).toHaveBeenCalledWith(...expected)

    - Expected
    + Received

    @@ -1,6 +1,10 @@
      Object {
    +   "application_metadata": Array [
    +     "cephfs",
    +     "rgw",
    +   ],
        "erasure_code_profile": "ecpMinimalMock",
        "pg_autoscale_mode": "off",
        "pg_num": 4,
        "pool": "ecPool",
        "pool_type": "erasure",,

    Number of calls: 1

      82 |     spyOn(taskWrapper, 'wrapTaskAroundCall').and.callThrough();
      83 |     component.submit();
    > 84 |     expect(poolService[poolServiceMethod]).toHaveBeenCalledWith(pool);
         |                                            ^
      85 |     expect(taskWrapper.wrapTaskAroundCall).toHaveBeenCalledWith({
      86 |       task: {
      87 |         name: taskName,

      at expectValidSubmit (src/app/ceph/pool/pool-form/pool-form.component.spec.ts:84:44)
      at expectEcSubmit (src/app/ceph/pool/pool-form/pool-form.component.spec.ts:1077:9)
      at src/app/ceph/pool/pool-form/pool-form.component.spec.ts:1122:9
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:409:30)
      at ProxyZoneSpec.Object.<anonymous>.ProxyZoneSpec.onInvoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:3830:43)
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:408:56)
      at Zone.Object.<anonymous>.Zone.run (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:169:47)
      at Object.wrappedFunc (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:4330:34)

  ● PoolFormComponent › submit - create › erasure coded pool › creates ec pool with ec_overwrite flag

    expect(spy).toHaveBeenCalledWith(...expected)

    - Expected
    + Received

    @@ -1,6 +1,10 @@
      Object {
    +   "application_metadata": Array [
    +     "cephfs",
    +     "rgw",
    +   ],
        "erasure_code_profile": "ecp1",
        "flags": Array [
          "ec_overwrites",
        ],
        "pg_autoscale_mode": "off",,

    Number of calls: 1

      82 |     spyOn(taskWrapper, 'wrapTaskAroundCall').and.callThrough();
      83 |     component.submit();
    > 84 |     expect(poolService[poolServiceMethod]).toHaveBeenCalledWith(pool);
         |                                            ^
      85 |     expect(taskWrapper.wrapTaskAroundCall).toHaveBeenCalledWith({
      86 |       task: {
      87 |         name: taskName,

      at expectValidSubmit (src/app/ceph/pool/pool-form/pool-form.component.spec.ts:84:44)
      at expectEcSubmit (src/app/ceph/pool/pool-form/pool-form.component.spec.ts:1077:9)
      at src/app/ceph/pool/pool-form/pool-form.component.spec.ts:1132:9
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:409:30)
      at ProxyZoneSpec.Object.<anonymous>.ProxyZoneSpec.onInvoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:3830:43)
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:408:56)
      at Zone.Object.<anonymous>.Zone.run (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:169:47)
      at Object.wrappedFunc (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:4330:34)

nizamial09 · 2023-08-17T05:24:32Z

@avanthakkar ping^

github-actions · 2023-10-16T06:02:17Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

github-actions · 2023-12-16T13:01:40Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

github-actions · 2024-01-15T15:01:31Z

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

avanthakkar · 2024-05-22T08:39:37Z

make-check failure

● PoolFormComponent › submit - create › erasure coded pool › minimum requirements without ECP to create ec pool

    expect(spy).toHaveBeenCalledWith(...expected)

    Expected: {"pg_autoscale_mode": "off", "pg_num": 4, "pool": "minECPool", "pool_type": "erasure"}

    Number of calls: 0

      82 |     spyOn(taskWrapper, 'wrapTaskAroundCall').and.callThrough();
      83 |     component.submit();
    > 84 |     expect(poolService[poolServiceMethod]).toHaveBeenCalledWith(pool);
         |                                            ^
      85 |     expect(taskWrapper.wrapTaskAroundCall).toHaveBeenCalledWith({
      86 |       task: {
      87 |         name: taskName,

      at expectValidSubmit (src/app/ceph/pool/pool-form/pool-form.component.spec.ts:84:44)
      at src/app/ceph/pool/pool-form/pool-form.component.spec.ts:1108:9
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:409:30)
      at ProxyZoneSpec.Object.<anonymous>.ProxyZoneSpec.onInvoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:3830:43)
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:408:56)
      at Zone.Object.<anonymous>.Zone.run (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:169:47)
      at Object.wrappedFunc (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:4330:34)

  ● PoolFormComponent › submit - create › erasure coded pool › creates ec pool with erasure coded profile

    expect(spy).toHaveBeenCalledWith(...expected)

    - Expected
    + Received

    @@ -1,6 +1,10 @@
      Object {
    +   "application_metadata": Array [
    +     "cephfs",
    +     "rgw",
    +   ],
        "erasure_code_profile": "ecpMinimalMock",
        "pg_autoscale_mode": "off",
        "pg_num": 4,
        "pool": "ecPool",
        "pool_type": "erasure",,

    Number of calls: 1

      82 |     spyOn(taskWrapper, 'wrapTaskAroundCall').and.callThrough();
      83 |     component.submit();
    > 84 |     expect(poolService[poolServiceMethod]).toHaveBeenCalledWith(pool);
         |                                            ^
      85 |     expect(taskWrapper.wrapTaskAroundCall).toHaveBeenCalledWith({
      86 |       task: {
      87 |         name: taskName,

      at expectValidSubmit (src/app/ceph/pool/pool-form/pool-form.component.spec.ts:84:44)
      at expectEcSubmit (src/app/ceph/pool/pool-form/pool-form.component.spec.ts:1077:9)
      at src/app/ceph/pool/pool-form/pool-form.component.spec.ts:1122:9
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:409:30)
      at ProxyZoneSpec.Object.<anonymous>.ProxyZoneSpec.onInvoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:3830:43)
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:408:56)
      at Zone.Object.<anonymous>.Zone.run (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:169:47)
      at Object.wrappedFunc (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:4330:34)

  ● PoolFormComponent › submit - create › erasure coded pool › creates ec pool with ec_overwrite flag

    expect(spy).toHaveBeenCalledWith(...expected)

    - Expected
    + Received

    @@ -1,6 +1,10 @@
      Object {
    +   "application_metadata": Array [
    +     "cephfs",
    +     "rgw",
    +   ],
        "erasure_code_profile": "ecp1",
        "flags": Array [
          "ec_overwrites",
        ],
        "pg_autoscale_mode": "off",,

    Number of calls: 1

      82 |     spyOn(taskWrapper, 'wrapTaskAroundCall').and.callThrough();
      83 |     component.submit();
    > 84 |     expect(poolService[poolServiceMethod]).toHaveBeenCalledWith(pool);
         |                                            ^
      85 |     expect(taskWrapper.wrapTaskAroundCall).toHaveBeenCalledWith({
      86 |       task: {
      87 |         name: taskName,

      at expectValidSubmit (src/app/ceph/pool/pool-form/pool-form.component.spec.ts:84:44)
      at expectEcSubmit (src/app/ceph/pool/pool-form/pool-form.component.spec.ts:1077:9)
      at src/app/ceph/pool/pool-form/pool-form.component.spec.ts:1132:9
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:409:30)
      at ProxyZoneSpec.Object.<anonymous>.ProxyZoneSpec.onInvoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:3830:43)
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:408:56)
      at Zone.Object.<anonymous>.Zone.run (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:169:47)
      at Object.wrappedFunc (node_modules/zone.js/bundles/zone-testing-bundle.umd.js:4330:34)

Fixed unit tests, passing locally!

avanthakkar · 2024-05-23T06:09:57Z

jenkins retest this please

afreen23 · 2024-05-23T11:40:55Z

src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html

+          <label class="cd-col-form-label required"
+                 for="applications">
+            <ng-container i18n>Applications</ng-container>
+            <cd-helper>


Let's use after the input tag here as well.

ceph/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.html

Lines 252 to 254 in ef9bf76

<cd-help-text>

<span i18n>The number of days that you want to specify for the default retention period that will be applied to new objects placed in this bucket.</span>

</cd-help-text>

We need to gradually move to this style, for all new things added and rest will be taken care by carbon

Thanks!

I've already addressed this here #51591. So once that's merged I can simple rebase this PR. Marking this PR as DNM for now and after #51591

@afreen23 I've merged #51591 and rebased this PR, can you review it again?

avanthakkar · 2024-05-27T07:13:50Z

After #51591

avanthakkar · 2024-05-27T09:32:35Z

jenkins test make check

avanthakkar · 2024-05-27T09:32:48Z

jenkins test dashboard

avanthakkar · 2024-05-27T09:32:55Z

jenkins test dashboard cephadm

avanthakkar · 2024-05-28T09:58:09Z

jenkins test dashboard

avanthakkar · 2024-05-28T09:58:15Z

jenkins test dashboard cephadm

avanthakkar · 2024-05-28T09:58:30Z

jenkins test make check arm64

avanthakkar · 2024-05-29T06:48:20Z

jenkins test dashboard

Fixes: https://tracker.ceph.com/issues/61238 Signed-off-by: avanthakkar <avanjohn@gmail.com>

sunilangadi2

@avanthakkar application is not mandatory for pool creation,
why are we making it as This field is required!?

avanthakkar · 2024-05-29T11:19:44Z

@avanthakkar application is not mandatory for pool creation, why are we making it as This field is required!?

Well you have to add some application anyways after creation else it'll show health_warn

sunilangadi2

yes it make sense from CLI as well

HEALTH_WARN 1 pool(s) do not have an application enabled; 4 pool(s) have non-power-of-two pg_num
[WRN] POOL_APP_NOT_ENABLED: 1 pool(s) do not have an application enabled
    application not enabled on pool 'rep_pool_mGnGGtgZwA'
    use 'ceph osd pool application enable <pool-name> <app-name>', where <app-name> is 'cephfs', 'rbd', 'rgw', or freeform for custom applications.```

cloudbehl · 2024-05-29T14:52:18Z

src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html

@@ -189,6 +194,9 @@
               title="Pools should be associated with an application tag"
               class="{{icons.warning}} icon-warning-color">
            </i>
+            <span class="invalid-feedback"
+                  *ngIf="!isApplicationsSelected"
+                  i18n>This field is required!</span>


Suggested change

i18n>This field is required!</span>

i18n>Application selection is required!</span>

cloudbehl · 2024-05-30T05:48:47Z

The validation sticks around even after selecting the application. Can you make sure once user selects the application this goes away.

Screencast.from.2024-05-30.11-17-14.webm

github-actions · 2024-06-07T14:28:56Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

avanthakkar requested a review from a team as a code owner May 18, 2023 12:15

avanthakkar requested review from pereman2 and aaSharma14 and removed request for a team May 18, 2023 12:15

github-actions bot added dashboard pybind labels May 18, 2023

pereman2 requested changes May 18, 2023

View reviewed changes

avanthakkar force-pushed the fix-pool-application branch from 85af8d1 to 634982b Compare May 18, 2023 12:32

avanthakkar requested a review from pereman2 May 18, 2023 12:33

Pegonzal approved these changes May 18, 2023

View reviewed changes

pereman2 approved these changes May 19, 2023

View reviewed changes

avanthakkar force-pushed the fix-pool-application branch from 634982b to 50b8cd8 Compare May 19, 2023 08:32

nizamial09 reviewed May 19, 2023

View reviewed changes

src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-data.ts Outdated Show resolved Hide resolved

avanthakkar force-pushed the fix-pool-application branch from 50b8cd8 to 874f8ec Compare May 19, 2023 09:41

github-actions bot added the stale label Oct 16, 2023

avanthakkar removed the stale label Oct 17, 2023

github-actions bot added the stale label Dec 16, 2023

github-actions bot closed this Jan 15, 2024

cloudbehl reopened this May 9, 2024

cloudbehl removed the stale label May 9, 2024

avanthakkar force-pushed the fix-pool-application branch from 874f8ec to 7f4ec99 Compare May 22, 2024 08:39

avanthakkar requested review from nizamial09 and Pegonzal May 22, 2024 08:39

avanthakkar force-pushed the fix-pool-application branch from 7f4ec99 to a6d1ba7 Compare May 23, 2024 09:13

afreen23 requested changes May 23, 2024

View reviewed changes

avanthakkar added the DNM label May 27, 2024

avanthakkar requested a review from afreen23 May 27, 2024 09:32

mgr/dashboard: make application field for pool creation mandatory

1feff25

Fixes: https://tracker.ceph.com/issues/61238 Signed-off-by: avanthakkar <avanjohn@gmail.com>

avanthakkar force-pushed the fix-pool-application branch from a6d1ba7 to 1feff25 Compare May 29, 2024 10:49

sunilangadi2 requested changes May 29, 2024

View reviewed changes

avanthakkar requested a review from sunilangadi2 May 29, 2024 11:19

sunilangadi2 approved these changes May 29, 2024

View reviewed changes

cloudbehl reviewed May 29, 2024

View reviewed changes

cloudbehl removed the DNM label May 29, 2024

github-actions bot added the needs-rebase label Jun 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mgr/dashboard: make application field for pool creation mandatory #51566

mgr/dashboard: make application field for pool creation mandatory #51566

avanthakkar commented May 18, 2023 •

edited

pereman2 left a comment

avanthakkar commented May 18, 2023

Pegonzal left a comment

avanthakkar commented May 19, 2023

nizamial09 left a comment

avanthakkar commented May 19, 2023

avanthakkar commented May 19, 2023

avanthakkar commented May 19, 2023

nizamial09 commented May 22, 2023

nizamial09 commented Aug 17, 2023

github-actions bot commented Oct 16, 2023

github-actions bot commented Dec 16, 2023

github-actions bot commented Jan 15, 2024

avanthakkar commented May 22, 2024

avanthakkar commented May 23, 2024

afreen23 May 23, 2024

avanthakkar May 27, 2024 •

edited

avanthakkar May 29, 2024

avanthakkar commented May 27, 2024

avanthakkar commented May 27, 2024

avanthakkar commented May 27, 2024

avanthakkar commented May 27, 2024

avanthakkar commented May 28, 2024

avanthakkar commented May 28, 2024

avanthakkar commented May 28, 2024

avanthakkar commented May 29, 2024

sunilangadi2 left a comment

avanthakkar commented May 29, 2024 •

edited

sunilangadi2 left a comment

cloudbehl May 29, 2024

cloudbehl commented May 30, 2024 •

edited

github-actions bot commented Jun 7, 2024

	<cd-help-text>
	<span i18n>The number of days that you want to specify for the default retention period that will be applied to new objects placed in this bucket.</span>
	</cd-help-text>

	i18n>This field is required!</span>
	i18n>Application selection is required!</span>

mgr/dashboard: make application field for pool creation mandatory #51566

Are you sure you want to change the base?

mgr/dashboard: make application field for pool creation mandatory #51566

Conversation

avanthakkar commented May 18, 2023 • edited

Contribution Guidelines

Checklist

pereman2 left a comment

Choose a reason for hiding this comment

avanthakkar commented May 18, 2023

Pegonzal left a comment

Choose a reason for hiding this comment

avanthakkar commented May 19, 2023

nizamial09 left a comment

Choose a reason for hiding this comment

avanthakkar commented May 19, 2023

avanthakkar commented May 19, 2023

avanthakkar commented May 19, 2023

nizamial09 commented May 22, 2023

nizamial09 commented Aug 17, 2023

github-actions bot commented Oct 16, 2023

github-actions bot commented Dec 16, 2023

github-actions bot commented Jan 15, 2024

avanthakkar commented May 22, 2024

avanthakkar commented May 23, 2024

afreen23 May 23, 2024

Choose a reason for hiding this comment

avanthakkar May 27, 2024 • edited

Choose a reason for hiding this comment

avanthakkar May 29, 2024

Choose a reason for hiding this comment

avanthakkar commented May 27, 2024

avanthakkar commented May 27, 2024

avanthakkar commented May 27, 2024

avanthakkar commented May 27, 2024

avanthakkar commented May 28, 2024

avanthakkar commented May 28, 2024

avanthakkar commented May 28, 2024

avanthakkar commented May 29, 2024

sunilangadi2 left a comment

Choose a reason for hiding this comment

avanthakkar commented May 29, 2024 • edited

sunilangadi2 left a comment

Choose a reason for hiding this comment

cloudbehl May 29, 2024

Choose a reason for hiding this comment

cloudbehl commented May 30, 2024 • edited

github-actions bot commented Jun 7, 2024

avanthakkar commented May 18, 2023 •

edited

avanthakkar May 27, 2024 •

edited

avanthakkar commented May 29, 2024 •

edited

cloudbehl commented May 30, 2024 •

edited