pkg/types: Push platform-specific types (AWS, etc.) into subdirs #657
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-bot
added
the
needs-rebase
openshift-ci-robot
added
the
size/XL
openshift-ci-robot
added
the
approved
wking
force-pushed
the
type-platform-subdirs
branch
from
a5324fa to
634165f
Compare
openshift-bot
removed
the
needs-rebase
|
Rebased around #600 with a5324fa -> 634165f. |
wking
force-pushed
the
type-platform-subdirs
branch
2 times, most recently
from
8426554 to
5299922
Compare
This decouples our platforms a bit and makes it easier to distinguish between platform-specific and platform-agnostic code. It also gives us much more compact struct names, since now we don't need to distinguish between many flavors of machine pool, etc. in a single package. I've also updated pkg/types/doc.go; pkg/types includes more than user-specified configuration since 78c3118 (pkg: add ClusterMetadata asset,type that can be used for destroy, 2018-09-25, openshift#324). I've also added OWNERS files for some OpenStack-specific directories that were missing them before. There's still more work to go in this direction (e.g. pushing default logic into subdirs), but this seems like a reasonable chunk.
wking
force-pushed
the
type-platform-subdirs
branch
from
5299922 to
6c5e904
Compare
/retest |
|
/lgtm |
|
/hold wait for comments from @flaper87 @tomassedovic |
openshift-ci-robot
added
the
do-not-merge/hold
|
filed openshift/cluster-network-operator#32 for this e2e-aws failure. Waiting for router to be created ...
NAME STATUS ROLES AGE VERSION
ip-10-0-1-38.ec2.internal Ready master 29m v1.11.0+d4cacc0
ip-10-0-128-19.ec2.internal Ready worker 24m v1.11.0+d4cacc0
ip-10-0-156-32.ec2.internal Ready worker 24m v1.11.0+d4cacc0
ip-10-0-174-183.ec2.internal Ready worker 23m v1.11.0+d4cacc0
ip-10-0-27-9.ec2.internal NotReady master 29m v1.11.0+d4cacc0
ip-10-0-46-179.ec2.internal Ready master 29m v1.11.0+d4cacc0
NAME DESIRED CURRENT READY UP-TO-DATE AVAILABLE NODE SELECTOR AGE
router-default 3 3 0 3 0 node-role.kubernetes.io/worker= 13s
Found router in openshift-ingress
Waiting for daemon set "router-default" rollout to finish: 0 of 3 updated pods are available...
Waiting for daemon set "router-default" rollout to finish: 1 of 3 updated pods are available...
Waiting for daemon set "router-default" rollout to finish: 2 of 3 updated pods are available...
daemon set "router-default" successfully rolled out
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
100 337 100 337 0 0 603 0 --:--:-- --:--:-- --:--:-- 605
namespace/openshift-telemetry created
cluster role "cluster-reader" added: "default"
error: error reading /tmp/cluster/telemeter-token: no such file or directory
error: no objects passed to apply
Error from server (Forbidden): deployments.apps "telemeter-client" is forbidden: caches not synchronized
which: no openshift-tests in (/usr/libexec/origin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin)
/bin/bash: line 112: !which: command not found
Running Suite: Extended
=======================
Random Seed: 1542060443 - Will randomize all specs
Will run 1251 specs
Running in parallel across 30 nodes
Nov 12 22:07:47.021: INFO: >>> kubeConfig: /tmp/admin.kubeconfig
Nov 12 22:07:47.023: INFO: Waiting up to 30m0s for all (but 0) nodes to be schedulable
Nov 12 22:07:47.408: INFO: Condition Ready of node ip-10-0-27-9.ec2.internal is false instead of true. Reason: KubeletNotReady, message: runtime network not ready: NetworkReady=false reason:NetworkPluginNotReady message:Network plugin returns error: cni config uninitialized
Nov 12 22:08:17.457: INFO: Condition Ready of node ip-10-0-27-9.ec2.internal is false instead of true. Reason: KubeletNotReady, message: runtime network not ready: NetworkReady=false reason:NetworkPluginNotReady message:Network plugin returns error: cni config uninitialized
Nov 12 22:08:47.491: INFO: Condition Ready of node ip-10-0-27-9.ec2.internal is false instead of true. Reason: KubeletNotReady, message: runtime network not ready: NetworkReady=false reason:NetworkPluginNotReady message:Network plugin returns error: cni config uninitialized
Nov 12 22:09:17.570: INFO: Condition Ready of node ip-10-0-27-9.ec2.internal is false instead of true. Reason: KubeletNotReady, message: runtime network not ready: NetworkReady=false reason:NetworkPluginNotReady message:Network plugin returns error: cni config uninitialized
Nov 12 22:09:47.499: INFO: Condition Ready of node ip-10-0-27-9.ec2.internal is false instead of true. Reason: KubeletNotReady, message: runtime network not ready: NetworkReady=false reason:NetworkPluginNotReady message:Network plugin returns error: cni config uninitialized
Nov 12 22:10:17.448: INFO: Condition Ready of node ip-10-0-27-9.ec2.internal is false instead of true. Reason: KubeletNotReady, message: runtime network not ready: NetworkReady=false reason:NetworkPluginNotReady message:Network plugin returns error: cni config uninitialized
Nov 12 22:10:47.453: INFO: Condition Ready of node ip-10-0-27-9.ec2.internal is false instead of true. Reason: KubeletNotReady, message: runtime network not ready:/retest |
|
/test e2e-openstack |
|
@abhinavdahiya thumbs up from my side. I've pulled the change locally, rebuilt the installer & deployed on OpenStack. There were no regressions. The |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, flaper87, wking 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 |
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/test all |
|
How can one remove an optional test from the check queue? The bot shouldn't block this patch on the openstack job, it's not a required job. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@wking: 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. |
|
/skip ^ this may get us past the e2e-openstack failure |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This decouples our platforms a bit and makes it easier to distinguish between platform-specific and platform-agnostic code. It also gives us much more compact struct names, since now we don't need to distinguish between many flavors of machine pool, etc. in a single package.
I've also added
OWNERSfiles for some OpenStack-specific directories that were missing them before.There's still more work to go in this direction (e.g. pushing default logic into subdirs), but this seems like a reasonable chunk.
CC @flaper87, @tomassedovic