controller: fix Redpanda 22.3 failing to start when license was loaded with 22.2 Redpanda #7312

graphcareful · 2022-11-16T14:43:09Z

Backport Required

release notes

Fix for bug that would prevent the start of redpanda if a license was loaded in a previous version

jcsp · 2022-11-16T15:08:20Z

tests/rptest/tests/license_upgrade_test.py

+        # Upload a license
+        self.admin.put_license(license)
+
+        # Update all nodes to newest version


This will need a followup to fix the version combinations: this is assuming that upgrades from 22.2->HEAD is what is wanted (i.e. on dev it will be skipping 22.3.1)).

- Ensures licenses can be read/written by cluster between versions

graphcareful · 2022-11-16T18:31:07Z

One unrelated test failure detected #6903:

test_id:    rptest.tests.schema_registry_test.SchemaRegistryAutoAuthTest.test_delete_subject
  | status:     FAIL
  | run time:   10.935 seconds
  |  
  |  
  | <NodeCrash docker-rp-8: ==25284==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x7f7282e57976 bp 0x7fff54148ae0 sp 0x7fff54148840 T0)
  | >
  | Traceback (most recent call last):
  | File "/usr/local/lib/python3.10/dist-packages/urllib3/connectionpool.py", line 670, in urlopen
  | httplib_response = self._make_request(
  | File "/usr/local/lib/python3.10/dist-packages/urllib3/connectionpool.py", line 426, in _make_request
  | six.raise_from(e, None)
  | File "<string>", line 3, in raise_from
  | File "/usr/local/lib/python3.10/dist-packages/urllib3/connectionpool.py", line 421, in _make_request
  | httplib_response = conn.getresponse()
  | File "/usr/lib/python3.10/http/client.py", line 1374, in getresponse
  | response.begin()
  | File "/usr/lib/python3.10/http/client.py", line 318, in begin
  | version, status, reason = self._read_status()
  | File "/usr/lib/python3.10/http/client.py", line 287, in _read_status
  | raise RemoteDisconnected("Remote end closed connection without"
  | http.client.RemoteDisconnected: Remote end closed connection without response

BenPope · 2022-11-16T20:56:52Z

/backport v22.3.x

jcsp · 2022-11-16T20:57:29Z

Was manually backported in #7318

vbotbuildovich · 2022-11-16T20:57:50Z

Failed to run cherry-pick command. I executed the below command:

git cherry-pick -x 84fa18eb59e5f779b8a131810d5408932ddb2202 1efea4c56872a6c0d7fe32e27d197613a8ea8c79

Workflow run logs.

emaxerrno · 2022-11-16T21:30:34Z

tests/rptest/tests/license_upgrade_test.py

@@ -108,3 +108,41 @@ def test_basic_upgrade(self):

        # Install license
        assert self.admin.put_license(license).status_code == 200
+
+
+class UpgradeMigratingLicenseVersion(RedpandaTest):


can we have a follow here on a Matrix annotation that does a to-from upgrades of the last 12 months upgrades to latest so ~4 or 5 upgrades.

cc: @graphcareful

@emaxerrno : AFAIK, we have upgrades from oldest supported release v21.11.x to intervening jumps to latest major's latest minor for the overall upgrade test.

I think what we might need here is to ensure that each such jump is upgrading from all-features-configured cluster to all-feature-in-next-version-cluster and so on.

And as @dotnwat pointed out having the binary compat testing would have caught this as well.

emaxerrno · 2022-11-16T21:32:35Z

src/v/security/license.h

@@ -62,7 +62,8 @@ inline std::ostream& operator<<(std::ostream& os, license_type lt) {
    return os;
 }

-struct license : serde::envelope<license, serde::version<1>> {
+struct license
+  : serde::envelope<license, serde::version<1>, serde::compat_version<0>> {


this is a cool fix.

@jcsp , @dotnwat and @mmaslankaprv - should we make compat_version<> explicit. No implicit versioning, in fact code should fail without compat-version?

maybe we check in with @felixguendling - seems like the issue was it compiled without serde::compat*

the issue was that the compat version had the wrong default. i think it is now explicit, iiuc.

jcsp · 2022-11-16T21:49:20Z

@jcsp , @dotnwat and @mmaslankaprv - should we make compat_version<> explicit. No implicit versioning, in fact code should fail without compat-version?

Yes, that was the immediate mitigation that I mentioned on slack, it is happening in #7314

can we have a follow here on a Matrix annotation that does a to-from upgrades of the last 12 months upgrades to latest so ~4 or 5 upgrades.

The need to avoid skipping intermediate versions is tracked in #7310

What we need in addition for that is a "fully loaded" upgrade test that tries to configure/enable all the features we can think of, and then traverses that series of supported upgrade steps. We have a bunch of upgrade tests but they tend to focus on a feature at as time, and some of them skip straight from some older version straight to HEAD.

emaxerrno · 2022-11-17T00:00:46Z

gosh you are all so smart and competent. thanks for already filing tix, etc.

github-actions bot added the area/redpanda label Nov 16, 2022

graphcareful requested a review from jcsp November 16, 2022 14:43

graphcareful force-pushed the license-fixes branch from 1f2b865 to 306d452 Compare November 16, 2022 14:44

security: Incorrect compat_ver in license struct

84fa18e

graphcareful force-pushed the license-fixes branch from 306d452 to 6bbaea7 Compare November 16, 2022 14:48

graphcareful requested a review from VladLazar November 16, 2022 15:01

graphcareful force-pushed the license-fixes branch 2 times, most recently from 52012fc to aec470e Compare November 16, 2022 15:02

jcsp reviewed Nov 16, 2022

View reviewed changes

jcsp previously approved these changes Nov 16, 2022

View reviewed changes

jcsp changed the title ~~Fix for bug where license could not be interpreted by newer versions of redpanda~~ controller: fix Redpanda 22.3 failing to start when license was loaded with 22.2 Redpanda Nov 16, 2022

jcsp mentioned this pull request Nov 16, 2022

serde: require explicit compat_version #7314

Merged

6 tasks

rptest: New license upgrade test

1efea4c

- Ensures licenses can be read/written by cluster between versions

graphcareful dismissed jcsp’s stale review via 1efea4c November 16, 2022 16:02

graphcareful force-pushed the license-fixes branch from aec470e to 1efea4c Compare November 16, 2022 16:02

graphcareful requested a review from jcsp November 16, 2022 17:12

jcsp approved these changes Nov 16, 2022

View reviewed changes

graphcareful added this to the v22.3.3 milestone Nov 16, 2022

This was referenced Nov 16, 2022

cluster: fix compat versions on structures #7316

Merged

[v22.3.x] compat decode fixes #7317

Closed

[v22.3.x] compat decode fixes #7318

Merged

jcsp merged commit 9db9127 into redpanda-data:dev Nov 16, 2022

emaxerrno reviewed Nov 16, 2022

View reviewed changes

graphcareful mentioned this pull request Nov 24, 2022

[v22.3.x] Backport license upgrade test #7485

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

controller: fix Redpanda 22.3 failing to start when license was loaded with 22.2 Redpanda #7312

controller: fix Redpanda 22.3 failing to start when license was loaded with 22.2 Redpanda #7312

graphcareful commented Nov 16, 2022 •

edited

jcsp Nov 16, 2022

graphcareful commented Nov 16, 2022 •

edited

BenPope commented Nov 16, 2022

jcsp commented Nov 16, 2022

vbotbuildovich commented Nov 16, 2022

emaxerrno Nov 16, 2022

emaxerrno Nov 16, 2022

piyushredpanda Nov 19, 2022

emaxerrno Nov 16, 2022

dotnwat Nov 16, 2022

jcsp commented Nov 16, 2022

emaxerrno commented Nov 17, 2022

controller: fix Redpanda 22.3 failing to start when license was loaded with 22.2 Redpanda #7312

controller: fix Redpanda 22.3 failing to start when license was loaded with 22.2 Redpanda #7312

Conversation

graphcareful commented Nov 16, 2022 • edited

Backport Required

release notes

jcsp Nov 16, 2022

Choose a reason for hiding this comment

graphcareful commented Nov 16, 2022 • edited

BenPope commented Nov 16, 2022

jcsp commented Nov 16, 2022

vbotbuildovich commented Nov 16, 2022

emaxerrno Nov 16, 2022

Choose a reason for hiding this comment

emaxerrno Nov 16, 2022

Choose a reason for hiding this comment

piyushredpanda Nov 19, 2022

Choose a reason for hiding this comment

emaxerrno Nov 16, 2022

Choose a reason for hiding this comment

dotnwat Nov 16, 2022

Choose a reason for hiding this comment

jcsp commented Nov 16, 2022

emaxerrno commented Nov 17, 2022

graphcareful commented Nov 16, 2022 •

edited

graphcareful commented Nov 16, 2022 •

edited