break dep on clientcmd in pkgs outside pkg oc #17357
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
the
size/M
| @@ -0,0 +1,24 @@ | |||
| package clientconfig | |||
There was a problem hiding this comment.
not in pkg/cmd I think that should die soon. pkg/client/cmd perhaps.
| @@ -27,7 +27,7 @@ func getOpenShiftClientEnvVars(options configapi.MasterConfig) ([]kapi.EnvVar, e | |||
| if err != nil { | |||
| return nil, err | |||
| } | |||
| return clientcmd.EnvVars( | |||
| return clientconfig.EnvVars( | |||
There was a problem hiding this comment.
This appears to be the only caller. Move it locally.
There was a problem hiding this comment.
Actually the entire getOpenShiftClientEnvVars needs refactoring. Queue up an issue to tidy it up.
There was a problem hiding this comment.
Opened issue to track this: #17397
juanvallejo
force-pushed
the
jvallejo/break-dep-on-clientcmd-in-pkgs-outside-pkg-oc
branch
from
2085686 to
b848907
Compare
juanvallejo
force-pushed
the
jvallejo/break-dep-on-clientcmd-in-pkgs-outside-pkg-oc
branch
2 times, most recently
from
dbb7ad0 to
27984fe
Compare
openshift-ci-robot
added
size/XL
juanvallejo
force-pushed
the
jvallejo/break-dep-on-clientcmd-in-pkgs-outside-pkg-oc
branch
from
a9efe89 to
9d92673
Compare
openshift-merge-robot
added
the
needs-rebase
juanvallejo
force-pushed
the
jvallejo/break-dep-on-clientcmd-in-pkgs-outside-pkg-oc
branch
from
9d92673 to
d832b50
Compare
juanvallejo
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
openshift-merge-robot
removed
the
needs-rebase
juanvallejo
force-pushed
the
jvallejo/break-dep-on-clientcmd-in-pkgs-outside-pkg-oc
branch
from
d832b50 to
f9a11f3
Compare
|
/retest |
| // currentMigrationRules returns a map that holds the history of recommended home directories used in previous versions. | ||
| // Any future changes to RecommendedHomeFile and related are expected to add a migration rule here, in order to make | ||
| // sure existing config files are migrated to their new locations properly. | ||
| func CurrentMigrationRules() map[string]string { |
There was a problem hiding this comment.
do the commands using this care about migration rules? Which ones are they?
There was a problem hiding this comment.
Our wrapper command for NewCmdConfig calls cmdconfig.NewOpenShiftClientConfigLoadingRules, which in turn calls currentMigrationRules
juanvallejo
force-pushed
the
jvallejo/break-dep-on-clientcmd-in-pkgs-outside-pkg-oc
branch
2 times, most recently
from
07f93e4 to
3dda8b9
Compare
|
/test extended_clusterup |
|
cc @deads2k |
This patch is a part of pull/17356 - it aims to break all dependencies between packages outside of `pkg/oc` and the `clientcmd` package. To achieve this, this patch creates a new package containing only the functions and objects found in `clientcmd` that are needed by its dependents outside of the `pkg/oc` subtree. Once this is done, all of the remaining logic (which should only be used by packages within `pkg/oc` is moved to `pkg/oc/cli/util/clientcmd` by pull/17356). This change acknowledges the possibility of having dependents for `pkg/cmd/util/clientconfig` within the `pkg/oc` subtree.
juanvallejo
force-pushed
the
jvallejo/break-dep-on-clientcmd-in-pkgs-outside-pkg-oc
branch
from
4900fe4 to
213f9a8
Compare
|
/test extended_clusterup |
|
/retest |
|
/retest |
|
/test extended_clusterup |
|
/retest |
|
/test crio |
|
/retest |
|
@juanvallejo start writing up flake issues for the different e2e issues. |
|
extended_clusterup flaked on #17548 |
|
crio test flaked on #14898 - error talking to yum |
|
/retest |
|
/test extended_conformance_install |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
@juanvallejo: The following test failed, say
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. |
|
Automatic merge from submit-queue. |
juanvallejo
deleted the
jvallejo/break-dep-on-clientcmd-in-pkgs-outside-pkg-oc
branch
Automatic merge from submit-queue (batch tested with PRs 17536, 17512). prevent references from origin to oc Builds on #17516 and #17357 After this, oc can be deleted from origin during the next kube bump. @juanvallejo missed one. I hacked it in. You'll want to pretty it up eventually. @eparis @smarterclayton after this merges, I think we're good to branch 3.8.
This patch is a part of pull/17356 - it aims to break all dependencies
between packages outside of
pkg/ocand theclientcmdpackage.To achieve this, this patch creates a new package containing only the
functions and objects found in
clientcmdthat are needed by itsdependents outside of the
pkg/ocsubtree. Once this is done, all ofthe remaining logic (which should only be used by packages within
pkg/ocis moved topkg/oc/cli/util/clientcmdby pull/17356).This change acknowledges the possibility of having dependents for
pkg/cmd/util/clientconfigwithin thepkg/ocsubtree.cc @deads2k @openshift/cli-review @liggitt