Issue 491: Handling config key name changes #492
Conversation
Signed-off-by: SrishT <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #492 +/- ##
==========================================
- Coverage 72.49% 71.62% -0.88%
==========================================
Files 15 15
Lines 3370 3411 +41
==========================================
Hits 2443 2443
- Misses 824 865 +41
Partials 103 103
Continue to review full report at Codecov.
|
Signed-off-by: SrishT <[email protected]>
| @@ -1065,8 +1069,10 @@ func (p *PravegaCluster) validateConfigMap() error { | |||
| } | |||
| if val, ok := p.Spec.Pravega.Options["bookkeeper.ledger.path"]; ok { | |||
| checkstring := fmt.Sprintf("-Dbookkeeper.ledger.path=%v", val) | |||
| eq := strings.Contains(configmap.Data["JAVA_OPTS"], checkstring) | |||
| if !eq { | |||
| eq_new := strings.Contains(configmap.Data["JAVA_OPTS"], checkstring) | |||
There was a problem hiding this comment.
In place of contains let's check if the configmap.Data["JAVA_OPTS"] is equal to the checkstring or not to avoid any false positives.
Please change this at all other places too.
There was a problem hiding this comment.
We cannot use the equals check here since this particular option is just one of the options within the list of java options for pravega.
There was a problem hiding this comment.
what I meant is let's think of a logic where we should check if specific option field value is equal to the changed value or not.
for eg think of a case where a user tries to change ledger path from ledgers/temp1 to ledgers this will pass our check even though the ledger path is changed.
| @@ -1065,8 +1069,10 @@ func (p *PravegaCluster) validateConfigMap() error { | |||
| } | |||
| if val, ok := p.Spec.Pravega.Options["bookkeeper.ledger.path"]; ok { | |||
| checkstring := fmt.Sprintf("-Dbookkeeper.ledger.path=%v", val) | |||
| eq := strings.Contains(configmap.Data["JAVA_OPTS"], checkstring) | |||
| if !eq { | |||
| eq_new := strings.Contains(configmap.Data["JAVA_OPTS"], checkstring) | |||
There was a problem hiding this comment.
what I meant is let's think of a logic where we should check if specific option field value is equal to the changed value or not.
for eg think of a case where a user tries to change ledger path from ledgers/temp1 to ledgers this will pass our check even though the ledger path is changed.
Signed-off-by: SrishT <[email protected]>
Signed-off-by: SrishT <[email protected]>
Signed-off-by: SrishT [email protected]
Change log description
Pravega webhook fails upgrade even when just the name of the configuration keys is modified as per the new naming convention (ref. pravega/pravega#4713)
Purpose of the change
Fixes #491
How to verify it
Upgrade pravega from version 0.7.0 to 0.8.0 and along with it also modify the name of the pravega option name from
controller.containerCounttocontroller.container.countwithout modifying its value. The upgrade should be allowed and you should not see the following error message