Bug 1749643: don't crash on non-hyphenated csc #249
Conversation
openshift-ci-robot
added
the
bugzilla/valid-bug
|
@gallettilance: This pull request references Bugzilla bug 1749643, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
size/S
pkg/migrator/migrator.go
Outdated
| if len(possibleCsName) > 1 { | ||
| return fmt.Sprintf("%s-%s", possibleCsName[1], "operators") | ||
| } | ||
| return return fmt.Sprintf("%s-%s", possibleCsName[0], "operators") |
There was a problem hiding this comment.
can you explain the return return?
pkg/migrator/migrator.go
Outdated
| if len(possibleCsName) > 1 { | ||
| return fmt.Sprintf("%s-%s", possibleCsName[1], "operators") | ||
| } | ||
| return return fmt.Sprintf("%s-%s", possibleCsName[0], "operators") |
There was a problem hiding this comment.
does this work in every case? should we have some more fundamental base case here instead?
There was a problem hiding this comment.
Maybe we can open a separate PR to improve the implementation of this function?
gallettilance
force-pushed
the
migration-bug-fix
branch
from
6c1f63e
to
a1febe7
Compare
|
/test e2e-aws-operator |
gallettilance
force-pushed
the
migration-bug-fix
branch
from
a1febe7
to
faf6de8
Compare
openshift-ci-robot
added
size/M
|
@kevinrizza @awgreene this is ready for another review |
gallettilance
force-pushed
the
migration-bug-fix
branch
3 times, most recently
from
5503b3b
to
984ae9b
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gallettilance, kevinrizza The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/retest |
|
/lgtm |
|
This looks good to me. |
Problem: non-hyphenated CatalogSourceConfigs cause the marketplace operator pod to crash during migration because the function that infers the CatalogSource name from the CatalogSourceConfig assumes the presence of a hyphen. Solution: Check that splitting a string on the hyphen character produces a list of length greater than 1.
gallettilance
force-pushed
the
migration-bug-fix
branch
from
984ae9b
to
63fdd2c
Compare
|
/lgtm |
|
/retest |
|
/retest |
5 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
@gallettilance: All pull requests linked via external trackers have merged. Bugzilla bug 1749643 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Problem:
non-hyphenated CatalogSourceConfigs cause the marketplace operator pod
to crash during migration because the function that infers the
CatalogSource name from the CatalogSourceConfig assumes the presence of
a hyphen.
Solution:
Check that splitting a string on the hyphen character produces a list of
length greater than 1.