Skip to content
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

Make occlient a superset of kclient #2643

Conversation

johnmcollier
Copy link
Member

@johnmcollier johnmcollier commented Feb 26, 2020

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind code-refactoring

What does does this PR do / why we need it:
This is an alternate draft PR to #2626, exploring an alternate approach to handling overlap between occlient and kclient.

Rather than keeping occlient and kclient functions completely separate, this PR adds an instance of kclient to the occlient instance, thus making occlient a superset of kclient. This works out nicely because occlient already has all of the fields that exist in kclient so we can use this as a chance to refactor and reduce some code duplication.

Additionally, this approach prevents us from having to pass in both occlient and kclient instances into the CLI context (and from having to pass in both to functions that require them).

As a summary, this PR does the following:

  • Moves the file sync code from the occlient package to the kclient package, so that it can be used with both odo "classic" push and odo "devfile" push.
  • Adds a kclient instance to the occlient object
    • Removes the following duplicated fields from occlient that already existed in kclient:
      • Namespace
      • KubeClient
      • ClientConfig
    • Updates existing code to use occlient.kclient where applicable

All unit tests passing:

johns-mbp-3:odo johncollier$ make test
go test  -race github.com/openshift/odo/cmd/cli-doc github.com/openshift/odo/cmd/odo github.com/openshift/odo/pkg/application github.com/openshift/odo/pkg/application/labels github.com/openshift/odo/pkg/auth github.com/openshift/odo/pkg/catalog github.com/openshift/odo/pkg/component github.com/openshift/odo/pkg/component/labels github.com/openshift/odo/pkg/config github.com/openshift/odo/pkg/debug github.com/openshift/odo/pkg/devfile github.com/openshift/odo/pkg/devfile/parser github.com/openshift/odo/pkg/devfile/versions github.com/openshift/odo/pkg/devfile/versions/1.0.0 github.com/openshift/odo/pkg/devfile/versions/common github.com/openshift/odo/pkg/kclient github.com/openshift/odo/pkg/log github.com/openshift/odo/pkg/log/fidget github.com/openshift/odo/pkg/machineoutput github.com/openshift/odo/pkg/notify github.com/openshift/odo/pkg/occlient github.com/openshift/odo/pkg/odo/cli github.com/openshift/odo/pkg/odo/cli/application github.com/openshift/odo/pkg/odo/cli/catalog github.com/openshift/odo/pkg/odo/cli/catalog/describe github.com/openshift/odo/pkg/odo/cli/catalog/list github.com/openshift/odo/pkg/odo/cli/catalog/search github.com/openshift/odo/pkg/odo/cli/catalog/util github.com/openshift/odo/pkg/odo/cli/component github.com/openshift/odo/pkg/odo/cli/component/ui github.com/openshift/odo/pkg/odo/cli/config github.com/openshift/odo/pkg/odo/cli/debug github.com/openshift/odo/pkg/odo/cli/login github.com/openshift/odo/pkg/odo/cli/logout github.com/openshift/odo/pkg/odo/cli/preference github.com/openshift/odo/pkg/odo/cli/project github.com/openshift/odo/pkg/odo/cli/service github.com/openshift/odo/pkg/odo/cli/service/ui github.com/openshift/odo/pkg/odo/cli/storage github.com/openshift/odo/pkg/odo/cli/ui github.com/openshift/odo/pkg/odo/cli/url github.com/openshift/odo/pkg/odo/cli/utils github.com/openshift/odo/pkg/odo/cli/version github.com/openshift/odo/pkg/odo/genericclioptions github.com/openshift/odo/pkg/odo/util github.com/openshift/odo/pkg/odo/util/completion github.com/openshift/odo/pkg/odo/util/experimental github.com/openshift/odo/pkg/odo/util/validation github.com/openshift/odo/pkg/preference github.com/openshift/odo/pkg/project github.com/openshift/odo/pkg/secret github.com/openshift/odo/pkg/service github.com/openshift/odo/pkg/storage github.com/openshift/odo/pkg/storage/labels github.com/openshift/odo/pkg/testingutil github.com/openshift/odo/pkg/testingutil/filesystem github.com/openshift/odo/pkg/url github.com/openshift/odo/pkg/url/labels github.com/openshift/odo/pkg/util github.com/openshift/odo/pkg/version
?   	github.com/openshift/odo/cmd/cli-doc	[no test files]
?   	github.com/openshift/odo/cmd/odo	[no test files]
ok  	github.com/openshift/odo/pkg/application	(cached)
ok  	github.com/openshift/odo/pkg/application/labels	(cached)
?   	github.com/openshift/odo/pkg/auth	[no test files]
ok  	github.com/openshift/odo/pkg/catalog	(cached)
ok  	github.com/openshift/odo/pkg/component	(cached)
ok  	github.com/openshift/odo/pkg/component/labels	(cached)
ok  	github.com/openshift/odo/pkg/config	(cached)
ok  	github.com/openshift/odo/pkg/debug	(cached)
ok  	github.com/openshift/odo/pkg/devfile	(cached)
ok  	github.com/openshift/odo/pkg/devfile/parser	(cached)
ok  	github.com/openshift/odo/pkg/devfile/versions	(cached)
?   	github.com/openshift/odo/pkg/devfile/versions/1.0.0	[no test files]
ok  	github.com/openshift/odo/pkg/devfile/versions/common	(cached)
ok  	github.com/openshift/odo/pkg/kclient	(cached)
?   	github.com/openshift/odo/pkg/log	[no test files]
?   	github.com/openshift/odo/pkg/log/fidget	[no test files]
?   	github.com/openshift/odo/pkg/machineoutput	[no test files]
ok  	github.com/openshift/odo/pkg/notify	(cached) [no tests to run]
ok  	github.com/openshift/odo/pkg/occlient	(cached)
?   	github.com/openshift/odo/pkg/odo/cli	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/application	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/catalog	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/catalog/describe	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/catalog/list	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/catalog/search	[no test files]
ok  	github.com/openshift/odo/pkg/odo/cli/catalog/util	(cached)
?   	github.com/openshift/odo/pkg/odo/cli/component	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/component/ui	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/config	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/debug	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/login	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/logout	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/preference	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/project	[no test files]
ok  	github.com/openshift/odo/pkg/odo/cli/service	(cached)
ok  	github.com/openshift/odo/pkg/odo/cli/service/ui	(cached)
ok  	github.com/openshift/odo/pkg/odo/cli/storage	(cached)
?   	github.com/openshift/odo/pkg/odo/cli/ui	[no test files]
?   	github.com/openshift/odo/pkg/odo/cli/url	[no test files]
ok  	github.com/openshift/odo/pkg/odo/cli/utils	(cached)
?   	github.com/openshift/odo/pkg/odo/cli/version	[no test files]
?   	github.com/openshift/odo/pkg/odo/genericclioptions	[no test files]
ok  	github.com/openshift/odo/pkg/odo/util	(cached)
ok  	github.com/openshift/odo/pkg/odo/util/completion	(cached)
ok  	github.com/openshift/odo/pkg/odo/util/experimental	(cached)
ok  	github.com/openshift/odo/pkg/odo/util/validation	(cached)
ok  	github.com/openshift/odo/pkg/preference	(cached)
ok  	github.com/openshift/odo/pkg/project	(cached)
ok  	github.com/openshift/odo/pkg/secret	(cached)
ok  	github.com/openshift/odo/pkg/service	(cached)
ok  	github.com/openshift/odo/pkg/storage	(cached)
ok  	github.com/openshift/odo/pkg/storage/labels	(cached)
ok  	github.com/openshift/odo/pkg/testingutil	(cached)
?   	github.com/openshift/odo/pkg/testingutil/filesystem	[no test files]
ok  	github.com/openshift/odo/pkg/url	(cached)
ok  	github.com/openshift/odo/pkg/url/labels	(cached)
ok  	github.com/openshift/odo/pkg/util	(cached)
?   	github.com/openshift/odo/pkg/version	[no test files]

Which issue(s) this PR fixes:

#2473 (partially)

How to test changes / Special notes to the reviewer:

Signed-off-by: John Collier <John.J.Collier@ibm.com>
Signed-off-by: John Collier <John.J.Collier@ibm.com>
@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. size/XL labels Feb 26, 2020
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign kadel
You can assign the PR to them by writing /assign @kadel in a comment when ready.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: John Collier <John.J.Collier@ibm.com>
Signed-off-by: John Collier <John.J.Collier@ibm.com>
@johnmcollier
Copy link
Member Author

/test v4.1-integration-e2e-benchmark

1 similar comment
@johnmcollier
Copy link
Member Author

/test v4.1-integration-e2e-benchmark

@openshift-ci-robot
Copy link
Collaborator

@johnmcollier: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/v4.1-integration-e2e-benchmark 8e5f8d1 link /test v4.1-integration-e2e-benchmark

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

@rm3l rm3l added the estimated-size/XL (40-60) Rough sizing for Epics. About 3 sprints of work for a person label Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. estimated-size/XL (40-60) Rough sizing for Epics. About 3 sprints of work for a person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants