-
Notifications
You must be signed in to change notification settings - Fork 287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Promote ConsolePlugin API version to v1 #440
Promote ConsolePlugin API version to v1 #440
Conversation
Signed-off-by: Yi Cai <[email protected]>
2439f4f
to
efa6ce5
Compare
/retest |
/retest |
/test v4.10-kuttl-parallel v4.9-kuttl-parallel |
/test v4.10-kuttl-sequential v4.8-kuttl-sequential |
/test v4.10-kuttl-sequential v4.8-kuttl-sequential v4.9-kuttl-parallel |
1 similar comment
/test v4.10-kuttl-sequential v4.8-kuttl-sequential v4.9-kuttl-parallel |
/test v4.10-kuttl-sequential |
6 similar comments
/test v4.10-kuttl-sequential |
/test v4.10-kuttl-sequential |
/test v4.10-kuttl-sequential |
/test v4.10-kuttl-sequential |
/test v4.10-kuttl-sequential |
/test v4.10-kuttl-sequential |
…perator into gitops-2646-update-dynamic-plugin-cr
….com/ciiay/gitops-operator into gitops-2646-update-dynamic-plugin-cr
/retest |
1 similar comment
/retest |
/retest-required |
/test v4.9-kuttl-parallel |
/retest-required |
…perator into gitops-2646-update-dynamic-plugin-cr
/retest |
/test all |
/retest |
/retest |
/retest |
/test v4.12-kuttl-sequential |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -113,7 +112,6 @@ func main() { | |||
registerComponentOrExit(mgr, operatorsv1alpha1.AddToScheme) | |||
registerComponentOrExit(mgr, argoapi.AddToScheme) | |||
registerComponentOrExit(mgr, configv1.AddToScheme) | |||
registerComponentOrExit(mgr, consolepluginv1.AddToScheme) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ciiay looks like we accidentally removed the line instead of updating it
if this line is not present the gitops operator will not be able to create the console plugin deployment at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaideepr97 Thanks for the review. This line was removed purposely. Because after updating to v1, the console plugin API is the same as console API, see Line 40, 41. And the console.AddToScheme
is already there in L 109
registerComponentOrExit(mgr, console.AddToScheme)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, interesting that we had both to begin with
thanks for clearing that up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaideepr97 Because when we started console plugin the plugin SDK feature was only on the console/v1alpha1 version, then later they updated it and the new feature in v1alpha1 got merged into the console/v1.
/test v4.11-kuttl-sequential v4.10-kuttl-sequential |
@ciiay @jaideepr97 Even though I mentioned earlier 1-042_validate_status_host is not a concern, now I feel that if we merge this PR, it will introduce this error in other PRs too. Do you think this code change has something to do with 1-042_validate_status_host test? |
/test v4.12-kuttl-sequential |
we tried looking at these test failures and we're pretty confident it's not related to this PR |
1-083_validate_apps_in_any_namespace and 1-050_validate_sso_config test failures can be ignored. But if you look at this board, 1-042_validate_status_host has been only failing for https://github.com/redhat-developer/gitops-operator/pull//481 and this PR which contain the same code changes |
/test v4.11-kuttl-sequential |
@ciiay: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
@varshab1210 But 1-042_validate_status_host not always fails, it passed in this taskrun: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/redhat-developer_gitops-operator/440/pull-ci-redhat-developer-gitops-operator-master-v4.12-kuttl-sequential/1661823444783730688 |
/test v4.12-kuttl-sequential |
Merging as administrator against the known failures |
…-dynamic-plugin-cr Promote ConsolePlugin API version to v1
Merge pull request #440 from ciiay/gitops-2646-update-dynamic-plugin-cr
…-dynamic-plugin-cr Promote ConsolePlugin API version to v1
What type of PR is this?
/kind bug
What does this PR do / why we need it:
ConsolePlugin API has been promoted to v1, we need to update the console plugin CR to pick up the changes.
It's related to GITOPS-2646 to enable i18n in upgraded version.
Which issue(s) this PR fixes:
Fixes GITOPS-2646
Test acceptance criteria:
How to test changes / Special notes to the reviewer:
Test ConsolePlugin works.