ceph-dencoder: RGW - Add missing types by NitzanMordhai · Pull Request #52198 · ceph/ceph

NitzanMordhai · 2023-06-27T05:34:02Z

Currently, ceph-dencoder lacks certain rgw types, preventing us from accurately checking the ceph corpus for encode-decode mismatches. This pull request aims to address this issue by adding the missing types to ceph-dencoder.

To successfully incorporate these types into ceph-dencoder, we need to introduce the necessary dump and generate_test_instances functions that was missing in some types. These functions are essential for proper encode and decode of the added types.

This PR will enhance the functionality of ceph-dencoder by including the missing types, enabling a comprehensive analysis of encode-decode consistency. With the addition of these types, we can ensure the robustness and correctness of the ceph corpus.

This update will significantly contribute to improving the overall reliability and accuracy of ceph-dencoder. It allows for a more comprehensive assessment of the encode-decode behavior, leading to enhanced data integrity and stability within the ceph ecosystem.

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

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

NitzanMordhai · 2023-06-27T06:25:21Z

jenkins test make check

NitzanMordhai · 2023-07-18T09:15:37Z

jenkins retest this please

cbodley

thanks! we were working on some of this in https://tracker.ceph.com/issues/54054 but didn't cover anything

cbodley · 2023-07-25T15:06:30Z

src/cls/rgw/cls_rgw_ops.h

+    f->open_array_section("keep_attr_prefixes_list");
+    for (auto iter = keep_attr_prefixes.begin(); iter != keep_attr_prefixes.end(); ++iter) {
+      f->dump_string("keep_attr_prefix", *iter);
+    }
+    f->close_section();


the helpers in src/common/ceph_json.h support std::list<T> already:

encode_json("keep_attr_prefixes", keep_attr_prefixes, f);

ok, missed that encode_json.. Changing it

cbodley · 2023-07-25T15:06:34Z

src/cls/rgw/cls_rgw_ops.h

+    f->open_array_section("entries");
+    for (std::list<rgw_cls_bi_entry>::const_iterator iter = entries.begin(); iter != entries.end(); ++iter) {
+      f->open_object_section("entry");
+      iter->dump(f);
+      f->close_section();
+    }
+    f->close_section();


encode_json("entries", entries, f);

cbodley · 2023-07-25T15:07:58Z

src/rgw/driver/rados/rgw_zone.h


  void dump(Formatter *f) const;
  void decode_json(JSONObj *obj);
+  //static void generate_test_instances(std::list<RGWSystemMetaObj*>& o);


cbodley · 2023-07-25T15:32:37Z

src/rgw/rgw_cors.cc

+  f->open_array_section("AllowedOrigin");
+  for (auto& origin : allowed_origins) {
+    f->dump_string("Origin", origin);
+  }
+  f->close_section();


does encode_json work for these lists too?

Yes, changed

cbodley · 2023-07-25T15:33:39Z

src/tools/ceph-dencoder/rgw_types.h

+//#include "rgw/driver/rados/rgw_zone.h"
+//TYPE_FEATUREFUL_NONDETERMINISTIC(RGWSystemMetaObj)


why commented out?

Abstract class, need to remove

cbodley · 2023-07-25T15:56:15Z

a 'make check' failure

**** reencode of /home/jenkins-build/build/workspace/ceph-pull-requests/ceph-object-corpus/archive/15.0.0-539-g191ab33faf/objects/rgw_placement_rule/276cee7e7a31943b9d27987b75241f8d resulted in a different dump ****
3c3
<     "storage_class": "STANDARD"
---
>     "storage_class": ""

cbodley · 2023-07-31T22:18:47Z

jenkins test make check

NitzanMordhai · 2023-08-01T07:08:25Z

reencode of /home/jenkins-build/build/workspace/ceph-pull-requests/ceph-object-corpus/archive/15.0.0-539-g191ab33faf/objects/rgw_placement_rule/276cee7e7a31943b9d27987b75241f8d resulted in a different dump
3c3
< "storage_class": "STANDARD"

"storage_class": ""

fixed the dump function to use the get storage class

ivancich · 2023-08-23T17:24:16Z

@NitzanMordhai -- this PR is different than others I've worked with in that your branch is not on your own fork of ceph/ceph but within ceph/ceph itself. I don't know if that's proper. @cbodley -- what are your thoughts?

NitzanMordhai · 2023-09-05T09:27:02Z

@NitzanMordhai -- this PR is different than others I've worked with in that your branch is not on your own fork of ceph/ceph but within ceph/ceph itself. I don't know if that's proper. @cbodley -- what are your thoughts?

Yes, I made a mistake with those PRs, and it won't happen again, I talked to @rzarzynski, and it looks like there is no easy way to change the base repository without closing and reopening new pr, which means losing all the comments.

…code-decode comparison Currently, ceph-dencoder lacks certain rgw types, preventing us from accurately checking the ceph corpus for encode-decode mismatches. This pull request aims to address this issue by adding the missing types to ceph-dencoder. To successfully incorporate these types into ceph-dencoder, we need to introduce the necessary `dump` and `generate_test_instances` functions that was missing in some types. These functions are essential for proper encode and decode of the added types. This PR will enhance the functionality of ceph-dencoder by including the missing types, enabling a comprehensive analysis of encode-decode consistency. With the addition of these types, we can ensure the robustness and correctness of the ceph corpus. This update will significantly contribute to improving the overall reliability and accuracy of ceph-dencoder. It allows for a more comprehensive assessment of the encode-decode behavior, leading to enhanced data integrity and stability within the ceph ecosystem. Fixes: https://tracker.ceph.com/issues/61788 Signed-off-by: Nitzan Mordechai <nmordech@redhat.com>

Signed-off-by: Nitzan Mordechai <nmordech@redhat.com>

ivancich · 2023-09-07T16:38:24Z

Passed QE:

NitzanMordhai requested a review from a team as a code owner June 27, 2023 05:34

github-actions bot added the rgw label Jun 27, 2023

NitzanMordhai assigned rzarzynski and neha-ojha and unassigned rzarzynski and neha-ojha Jun 27, 2023

NitzanMordhai requested review from neha-ojha and rzarzynski June 27, 2023 05:34

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-rgw-extend-types-available branch 2 times, most recently from 888de97 to c48707b Compare June 28, 2023 05:05

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-rgw-extend-types-available branch from c48707b to 84dc3a0 Compare July 17, 2023 12:38

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-rgw-extend-types-available branch from 84dc3a0 to b230bba Compare July 24, 2023 12:29

rzarzynski requested a review from cbodley July 24, 2023 13:50

cbodley reviewed Jul 25, 2023

View reviewed changes

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-rgw-extend-types-available branch from b230bba to f4d65da Compare July 26, 2023 06:22

NitzanMordhai requested a review from cbodley July 26, 2023 06:22

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-rgw-extend-types-available branch from f4d65da to ceade3f Compare July 26, 2023 06:32

cbodley approved these changes Jul 31, 2023

View reviewed changes

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-rgw-extend-types-available branch from ceade3f to 35dd76c Compare August 1, 2023 07:07

NitzanMordhai added the needs-qa label Aug 6, 2023

NitzanMordhai changed the title ~~ceph-dencoder: Add missing rgw types to ceph-dencoder for accurate encode-decode comparison~~ ceph-dencoder: RGW - Add missing types Aug 6, 2023

ivancich added the wip-eric-testing-1 for ivancich testing label Aug 23, 2023

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-rgw-extend-types-available branch from 35dd76c to d7c7b00 Compare September 6, 2023 09:00

NitzanMordhai added 2 commits September 7, 2023 07:53

submodule: update ceph-object-corpus submodule

94021bb

Signed-off-by: Nitzan Mordechai <nmordech@redhat.com>

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-rgw-extend-types-available branch from 82f23e9 to 94021bb Compare September 7, 2023 11:28

ivancich merged commit cbc4c2b into main Sep 7, 2023

ivancich deleted the wip-nitzan-ceph-dencoder-rgw-extend-types-available branch September 7, 2023 16:40

ivancich removed needs-qa wip-eric-testing-1 for ivancich testing labels Sep 7, 2023

		//#include "rgw/driver/rados/rgw_zone.h"
		//TYPE_FEATUREFUL_NONDETERMINISTIC(RGWSystemMetaObj)

Conversation

NitzanMordhai commented Jun 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

NitzanMordhai commented Jun 27, 2023

Uh oh!

NitzanMordhai commented Jul 18, 2023

Uh oh!

cbodley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbodley commented Jul 25, 2023

Uh oh!

cbodley commented Jul 31, 2023

Uh oh!

NitzanMordhai commented Aug 1, 2023

**** reencode of /home/jenkins-build/build/workspace/ceph-pull-requests/ceph-object-corpus/archive/15.0.0-539-g191ab33faf/objects/rgw_placement_rule/276cee7e7a31943b9d27987b75241f8d resulted in a different dump **** 3c3 < "storage_class": "STANDARD"

Uh oh!

ivancich commented Aug 23, 2023

Uh oh!

NitzanMordhai commented Sep 5, 2023

Uh oh!

ivancich commented Sep 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

NitzanMordhai commented Jun 27, 2023 •

edited

Loading

reencode of /home/jenkins-build/build/workspace/ceph-pull-requests/ceph-object-corpus/archive/15.0.0-539-g191ab33faf/objects/rgw_placement_rule/276cee7e7a31943b9d27987b75241f8d resulted in a different dump
3c3
< "storage_class": "STANDARD"