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.

Sign up for GitHub

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

Jump to bottom

Set LC_ALL in artifacts so that aws cli would convert unicode correctly #6536

Merged
merged 2 commits into from
Jan 2, 2020
Merged

Set LC_ALL in artifacts so that aws cli would convert unicode correctly #6536

merged 2 commits into from
Jan 2, 2020

Conversation

vrutkovs
Copy link
Member

@vrutkovs vrutkovs commented Jan 2, 2020 •
edited
Loading

This ensures LC_ALL is set so that aws ec2 get-console-output would save console output correctly. Second commit would ensure only text is saved

@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. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 2, 2020
@vrutkovs vrutkovs changed the title WIP Fix get-console-output Set LC_ALL in artifacts so that aws cli would convert unicode correctly Jan 2, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 2, 2020
@vrutkovs
Copy link
Member Author

vrutkovs commented Jan 2, 2020

/cc @wking

@vrutkovs
Copy link
Member Author

vrutkovs commented Jan 2, 2020

/retest

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jan 2, 2020 •
edited
Loading

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

Test name Commit Details Rerun command
ci/rehearse/openshift-priv/ci-experiment-origin/master/e2e-cmd 6ec5bf3 link /test pj-rehearse
ci/prow/pj-rehearse 6ec5bf3 link /test pj-rehearse

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.

@wking
Copy link
Member

wking commented Jan 2, 2020

Looks great:

$ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_release/6536/rehearse-6536-pull-ci-cri-o-cri-o-release-1.13-e2e-aws/4/artifacts/e2e-aws/nodes/i-0230ada7993e42afa/console | head -n3
i-0230ada7993e42afa	 Started RPM-OSTree System Management Daemon.
[  OK  ] Started Log RPM-OSTree Booted Deployment Status To Journal.
[  OK  ] Started Open Container Initiative Daemon.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 2, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vrutkovs, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 2, 2020
@vrutkovs vrutkovs mentioned this pull request Jan 2, 2020
Merged
@openshift-merge-robot openshift-merge-robot merged commit 0c28d6c into openshift:master Jan 2, 2020
@openshift-ci-robot
Copy link
Contributor

@vrutkovs: Updated the following 3 configmaps:

  • prow-job-cluster-launch-installer-e2e configmap in namespace ci at cluster default using the following files:
    • key cluster-launch-installer-e2e.yaml using file ci-operator/templates/openshift/installer/cluster-launch-installer-e2e.yaml
  • prow-job-cluster-launch-installer-e2e configmap in namespace ci-stg at cluster default using the following files:
    • key cluster-launch-installer-e2e.yaml using file ci-operator/templates/openshift/installer/cluster-launch-installer-e2e.yaml
  • prow-job-cluster-launch-installer-e2e configmap in namespace ci at cluster ci/api-build01-ci-devcluster-openshift-com:6443 using the following files:
    • key cluster-launch-installer-e2e.yaml using file ci-operator/templates/openshift/installer/cluster-launch-installer-e2e.yaml

In response to this:

This ensures LC_ALL is set so that aws ec2 get-console-output would save console output correctly. Second commit would ensure only text is saved

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.

@petr-muller petr-muller mentioned this pull request Jan 3, 2020
Merged
@wking wking mentioned this pull request Jun 17, 2020
Merged
wking added a commit to wking/openshift-release that referenced this pull request Jun 18, 2020
@wking
gather/aws/console: Add a step to gather console logs on AWS
390d6be 
History of this logic:

* Initially landed for nodes in e102a16
  (ci-operator/templates/openshift/installer/cluster-launch-installer-e2e:
  Gather node console logs on AWS, 2019-12-02, openshift#6189).

* Grew --text in 6ec5bf3 (installer artifacts: keep text version of
  instance output, 2020-01-02, openshift#6536).

* Grew machine handling in a469f53
  (ci-operator/templates/openshift/installer/cluster-launch-installer-e2e:
  Gather console logs from Machine providerID too, 2020-01-29, openshift#6906).

* The node-provider-IDs creation was ported to steps in e2fd5c7
  (step-registry: update deprovision step, 2020-01-30, openshift#6708), but
  without any consumer for the collected file.

The aws-instance-ids.txt injection allows install-time steps to
register additional instances for later console collection (for a
proxy instance, bootstrap instance, etc.).

Approvers are:

* Myself and Vadim, who have touched this logic in the past.
* Alberto, representing the machine-API space that needs console logs
  to debug failed-to-boot issues.
* Colin, representing the RHCOS/machine-config space that needs
  console logs to debug RHCOS issues.

TMPDIR documentation is based on POSIX [1].

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jstuever jstuever Awaiting requested review from jstuever

@patrickdillon patrickdillon Awaiting requested review from patrickdillon

@wking wking Awaiting requested review from wking

Assignees

@wking wking

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vrutkovs @openshift-ci-robot @wking @openshift-merge-robot