ceph-dencoder: COMMON - Add missing types #52210

NitzanMordhai · 2023-06-27T13:01:16Z

Currently, ceph-dencoder lacks certain common 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

rzarzynski

LGTM. Just nits.

rzarzynski · 2023-08-08T13:37:57Z

src/auth/Auth.h

@@ -59,6 +59,16 @@ struct EntityAuth {
      decode(pending_key, bl);
    }
  }
+  void dump(ceph::Formatter *f) const {
+    f->dump_object("key", key);
+    for (auto p = caps.begin(); p != caps.end(); ++p) {


nit: let's use the range-based for loop.

rzarzynski · 2023-08-08T13:38:49Z

src/auth/Auth.h

@@ -95,6 +105,21 @@ struct AuthCapsInfo {
    allow_all = (bool)a;
    decode(caps, bl);
  }
+  void dump(ceph::Formatter *f) const {
+    f->dump_bool("allow_all", allow_all);
+    for (auto p = caps.begin(); !p.end(); ++p) {


nit: https://en.cppreference.com/w/cpp/language/range-for

rzarzynski · 2023-08-08T13:39:27Z

src/auth/Auth.h

@@ -295,6 +349,23 @@ struct RotatingSecrets {
  }

  void dump();
+  void dump(ceph::Formatter *f) const {
+    f->open_array_section("secrets");
+    for (auto& p : secrets) {


nit: const auto& + structured bindings.

rzarzynski · 2023-08-08T16:11:25Z

src/auth/Auth.h

+    ls.push_back(new ExpiringCryptoKey);
+    ls.push_back(new ExpiringCryptoKey);
+    ls.back()->key.set_secret(CEPH_CRYPTO_AES, bufferptr("1234567890123456", 16),
+      utime_t(123, 456));


nit: how about moving all the args to the newline?

rzarzynski · 2023-08-08T16:11:48Z

src/auth/Crypto.cc

+  ls.push_back(new CryptoKey);
+  ls.back()->type = CEPH_CRYPTO_AES;
+  ls.back()->set_secret(CEPH_CRYPTO_AES, bufferptr("1234567890123456", 16),
+      utime_t(123, 456));


rzarzynski · 2023-08-08T16:23:20Z

src/cls/version/cls_version_ops.h

+  void dump(ceph::Formatter *f) const {
+    f->dump_object("objv", objv);
+    f->open_array_section("conds");
+    for (auto& i : conds) {


rzarzynski · 2023-08-08T16:24:08Z

src/cls/version/cls_version_types.cc

@@ -6,12 +6,6 @@
 #include "common/ceph_json.h"


-void obj_version::dump(ceph::Formatter *f) const


~~I'm not seeing it's being moved; really looks like a deletion.~~

rzarzynski · 2023-08-08T16:28:48Z

src/common/entity_name.cc

@@ -29,6 +29,25 @@ const std::array<EntityName::str_to_entity_type_t, 6> EntityName::STR_TO_ENTITY_
  { CEPH_ENTITY_TYPE_CLIENT, "client" },
 }};

+void EntityName::


nit: let's not split this.

rzarzynski · 2023-08-08T16:29:22Z

src/common/entity_name.cc

+  f->dump_string("id", id);
+}
+
+void EntityName::


rzarzynski · 2023-08-08T16:30:17Z

src/messages/MClientRequest.h

@@ -59,6 +59,17 @@ struct SnapPayload {
    decode(metadata, iter);
    DECODE_FINISH(iter);
  }
+  void dump(ceph::Formatter *f) const {
+    for (auto &i : metadata) {


rzarzynski · 2023-08-08T16:31:33Z

jenkins test api

NitzanMordhai · 2023-09-07T07:43:49Z

jenkins retest this please

NitzanMordhai · 2023-09-10T07:09:15Z

jenkins test make check

… encode-decode comparison Currently, ceph-dencoder lacks certain common 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>

NitzanMordhai · 2023-09-13T09:14:05Z

jenkins retest this please

github-actions · 2023-11-12T10:01:42Z

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.

athanatos · 2023-11-13T03:07:14Z

@NitzanMordhai Looks like @rzarzynski approved this PR -- is it still relevant?

rzarzynski · 2023-11-13T14:34:18Z

src/cls/version/cls_version_types.h

@@ -53,7 +53,11 @@ struct obj_version {
            tag.compare(v.tag) == 0);
  }

-  void dump(ceph::Formatter *f) const;
+  void dump(ceph::Formatter *f) const {


ACK (for the comment on obj_version from the last round).

rzarzynski · 2023-11-13T14:40:44Z

@athanatos: Yup, we want those changes.

NitzanMordhai · 2023-12-13T09:14:18Z

rados approve

Unrelated failures:

Details:

['7487680', '7487836'] - qa/workunits/post-file.sh: Couldn't create directory
['7487541', '7487610', '7487750', '7487751', '7487683', '7487822'] - centos 9 testing reveals rocksdb "Leak_StillReachable" memory leak in mons
['7487677', '7487816', '7487532'] - mgr: 'ceph rgw realm bootstrap' fails with KeyError: 'realm_name'
['7487804', '7487806', '7487647'] - rados/singleton-nomsgr: test failing from "Health check failed: 1 full osd(s) (OSD_FULL)" and "Health check failed: 1 filesystem is offline (MDS_ALL_DOWN)"
['7487724', '7487568'] - mgr/dashboard: fix e2e for dashboard v3
['7487579', '7487739'] - cephtest bash -c ceph_test_lazy_omap_stats

cbodley · 2024-03-06T17:56:39Z

has any of this been backported to reef? ultimately we want to enable the 18.2.0 corpus there too so we can catch encoding regressions in reef backports

NitzanMordhai requested a review from a team as a code owner June 27, 2023 13:01

github-actions bot added common rgw labels Jun 27, 2023

NitzanMordhai requested review from ljflores, neha-ojha and rzarzynski June 27, 2023 13:01

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-extend-common-types-available branch from 36b64aa to 1508cc3 Compare June 28, 2023 05:55

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-extend-common-types-available branch from 1508cc3 to 6a47b20 Compare July 17, 2023 12:29

NitzanMordhai requested a review from a team as a code owner July 17, 2023 12:29

github-actions bot added bluestore cephfs Ceph File System core labels Jul 17, 2023

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-extend-common-types-available branch 2 times, most recently from b45e950 to 6814743 Compare July 18, 2023 11:29

Matan-B mentioned this pull request Aug 3, 2023

ceph-dencoder: OSD - Add missing types #52482

Closed

14 tasks

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

rzarzynski approved these changes Aug 8, 2023

View reviewed changes

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-extend-common-types-available branch 2 times, most recently from 07a181b to 3ac3726 Compare August 13, 2023 07:14

NitzanMordhai requested a review from rzarzynski August 13, 2023 07:14

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-extend-common-types-available branch from 3ac3726 to 0959234 Compare September 6, 2023 08:58

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-extend-common-types-available branch 5 times, most recently from 5af2ca6 to ee52e0d Compare September 10, 2023 05:14

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-extend-common-types-available branch from ee52e0d to 3104663 Compare September 10, 2023 12:01

NitzanMordhai requested review from cloudbehl and aaSharma14 September 12, 2023 05:44

github-actions bot added api-change cephadm crimson dashboard documentation pybind tests labels Sep 12, 2023

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-extend-common-types-available branch 4 times, most recently from bd937af to 28716d7 Compare September 13, 2023 07:02

NitzanMordhai added 2 commits September 13, 2023 08:29

submodule: update ceph-object-corpus submodule

c64501d

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

NitzanMordhai force-pushed the wip-nitzan-ceph-dencoder-extend-common-types-available branch from 28716d7 to c64501d Compare September 13, 2023 08:30

github-actions bot added the stale label Nov 12, 2023

github-actions bot removed the stale label Nov 13, 2023

rzarzynski approved these changes Nov 13, 2023

View reviewed changes

rzarzynski added the needs-qa label Nov 13, 2023

ljflores added the wip-yuri8-testing label Dec 6, 2023

yuriw merged commit bf0b102 into main Dec 13, 2023
10 of 13 checks passed

yuriw deleted the wip-nitzan-ceph-dencoder-extend-common-types-available branch December 13, 2023 15:40

NitzanMordhai mentioned this pull request Mar 6, 2024

dencoder: update ceph-object-corpus submodule for 18.2.0 coverage #54735

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ceph-dencoder: COMMON - Add missing types #52210

ceph-dencoder: COMMON - Add missing types #52210

NitzanMordhai commented Jun 27, 2023 •

edited

rzarzynski left a comment

rzarzynski Aug 8, 2023

NitzanMordhai Aug 10, 2023

rzarzynski Aug 8, 2023

NitzanMordhai Aug 10, 2023

rzarzynski Aug 8, 2023

NitzanMordhai Aug 10, 2023

rzarzynski Aug 8, 2023

rzarzynski Aug 8, 2023

NitzanMordhai Aug 10, 2023

rzarzynski Aug 8, 2023

rzarzynski Aug 8, 2023

rzarzynski Aug 8, 2023

NitzanMordhai Aug 10, 2023

rzarzynski Aug 8, 2023

rzarzynski Aug 8, 2023

NitzanMordhai Aug 10, 2023

rzarzynski commented Aug 8, 2023

NitzanMordhai commented Sep 7, 2023

NitzanMordhai commented Sep 10, 2023

NitzanMordhai commented Sep 13, 2023

github-actions bot commented Nov 12, 2023

athanatos commented Nov 13, 2023

rzarzynski Nov 13, 2023

rzarzynski commented Nov 13, 2023

NitzanMordhai commented Dec 13, 2023

cbodley commented Mar 6, 2024

		@@ -6,12 +6,6 @@
		#include "common/ceph_json.h"


		void obj_version::dump(ceph::Formatter *f) const

ceph-dencoder: COMMON - Add missing types #52210

ceph-dencoder: COMMON - Add missing types #52210

Conversation

NitzanMordhai commented Jun 27, 2023 • edited

Contribution Guidelines

Checklist

rzarzynski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rzarzynski commented Aug 8, 2023

NitzanMordhai commented Sep 7, 2023

NitzanMordhai commented Sep 10, 2023

NitzanMordhai commented Sep 13, 2023

github-actions bot commented Nov 12, 2023

athanatos commented Nov 13, 2023

Choose a reason for hiding this comment

rzarzynski commented Nov 13, 2023

NitzanMordhai commented Dec 13, 2023

cbodley commented Mar 6, 2024

NitzanMordhai commented Jun 27, 2023 •

edited