openshift: record etcd logs in test runs #2996
Conversation
test/e2e/runner/cli_provisioner.go
Outdated
| } | ||
| master := fmt.Sprintf("%s@%s", eng.ClusterDefinition.Properties.LinuxProfile.AdminUsername, kubeConfig.GetServerName()) | ||
|
|
||
| services := []string{"etcd.service"} |
There was a problem hiding this comment.
Soon this will run as a pod. Can you try to separate the logic based on the orchestrator version?
|
@CecileRobertMichon please merge |
|
/lgtm We should retrieve logs from the rest of the control plane in a follow-up. I am also looking into deploying all of our components with increased log verbosity as part of the e2e test deployment. |
|
@Kargakis: changing LGTM is restricted to assignees, and only Azure org members may be assigned issues. 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. |
test/e2e/kubernetes/config.go
Outdated
| @@ -45,5 +45,9 @@ func GetConfig() (*Config, error) { | |||
| // GetServerName returns the server for the given config in an sshable format | |||
| func (c *Config) GetServerName() string { | |||
| s := c.Clusters[0].ClusterInfo.Server | |||
| return strings.Split(s, "://")[1] | |||
| s = strings.Split(s, "://")[1] | |||
| if strings.ContainsRune(s, ':') { | |||
There was a problem hiding this comment.
should this not be a parsable URL?
@Kargakis unclear if you want me to open an issue or if you're working on it already? |
Working on bumping the loglevel now, once this PR merges I will work on getting logs from the rest of the services. |
Codecov Report
@@ Coverage Diff @@
## master #2996 +/- ##
==========================================
- Coverage 51.59% 50.75% -0.85%
==========================================
Files 97 97
Lines 14834 14682 -152
==========================================
- Hits 7654 7452 -202
- Misses 6482 6533 +51
+ Partials 698 697 -1
Continue to review full report at Codecov.
|
|
@CecileRobertMichon please merge |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CecileRobertMichon 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 |
|
|
@Kargakis: thanks for the catch - updated |
|
lgtm, @CecileRobertMichon please merge |
|
/lgtm |
|
@Kargakis: adding LGTM is restricted to approvers and reviewers in OWNERS files. 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. |
|
/lgtm |
|
@Kargakis: adding LGTM is restricted to approvers and reviewers in OWNERS files. 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. |
|
@acs-bot I keep forgetting @CecileRobertMichon ping |
|
@jim-minter @Kargakis I restarted the openshift e2e twice and have not been able to get a single successful.I would like to get at least one e2e run since this PR is changing the test itself. It might be worth considering disabling the test until it is more reliable since there is little in a test that is red more often than it is green what do you think? |
|
Hey @CecileRobertMichon we're also frustrated by the testing situation. Two issues are that we're spread thin right now, and that we also don't have all the gating we need in place upstream on the origin and openshift-ansible repos so we're perpetually chasing breaks there. My personal preference is to leave the tests here enabled and red rather than disabled - we will get to the flakes and get them green. |
No description provided.