Skip to content

WIP: Tests for #540 #542

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
Closed
mtrmac wants to merge 5 commits into from
Closed

WIP: Tests for #540 #542

mtrmac wants to merge 5 commits into from

Conversation

@mtrmac
Copy link
Contributor

@mtrmac mtrmac commented Mar 12, 2019

This attempts to test #540 by hard-coding a permission-required pause image, per #537.

First iteration reverts both of the fixes, so that I can see what the failure looks like; then I’ll drop the revert to test the code from #540.

@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 12, 2019

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 12, 2019
@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 13, 2019

/test all

@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 13, 2019

Well, that was underwhelming… artifacts/e2e-aws-op/bootstrap/crio.service is empty.

Anyway, the closest to an error report is

level=fatal msg="waiting for Kubernetes API: context deadline exceeded"
2019/03/13 13:44:10 Container setup in pod e2e-aws-op failed, exit code 1, reason Error

let’s see if actually including the fix improves things.

@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 13, 2019

/test all

@mtrmac mtrmac force-pushed the pause_image_auth_file-tests branch from c00fe2c to 9347cad Compare March 13, 2019 14:09
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 13, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mtrmac
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: ashcrow

If they are not already assigned, you can assign the PR to them by writing /assign @ashcrow in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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 size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 13, 2019
@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 13, 2019

/retest

@cgwalters
Copy link
Member

cgwalters commented Mar 13, 2019

OK, so at least that last e2e-aws-op run failed with a symptom that could very well be caused by the fact this patch didn't work.

#530
would help us debug this.

At the moment whenever I have to debug something like this I end up using libvirt since you can ssh to the bootstrap node easily, get the console logs from the masters, etc.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 16, 2019
mtrmac added 4 commits March 19, 2019 19:42
@mtrmac
vendor: bump github.com/kubernetes-sigs/cri-o
9e9bc7d 
... to include ImageConfig.PauseImageAuthFile, so that
updateCRIOConfig does not remove the value.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac
Configure CRI-O to use the Kubelet pull secret for the pause image
1ecc9c3 
The path is hard-coded here as well as in the templates that
create the file, in templates/*/00-*/_base/files/pull-secret.yaml .

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac
Revert "controller: Symlink /root/.docker/config.json to kubelet auth"
4e82446 
This reverts commit 8b08dfb.

We now configure CRI-O to use the Kubelet's path explicitly, so this
symlink should no longer be necessary.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac
Merge branch 'pause_image_auth_file' into HEAD
bb71965
@mtrmac mtrmac force-pushed the pause_image_auth_file-tests branch from 9347cad to 7d0800d Compare March 19, 2019 18:51
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 19, 2019
@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 19, 2019

(Tests succeeded, both in #540 and here, after rebasing; while I am waiting for a machine to investigate possible failures in detail, let’s try the “revert both fixes” experiment again.)

@mtrmac mtrmac force-pushed the pause_image_auth_file-tests branch from e1b15af to 7d0800d Compare March 19, 2019 23:23
@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 19, 2019

(Failures with both fixes reverted are the same as last time:

)

@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 19, 2019 •
edited
Loading

(https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/openshift_machine-config-operator/542/pull-ci-openshift-machine-config-operator-master-e2e-aws/2637 seems to be AWS RequestLimitExceeded.)

@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 20, 2019

/test e2e-aws

@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 20, 2019

/retest

4 similar comments
@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 20, 2019

/retest

@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 20, 2019

/retest

@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 20, 2019

/retest

@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 20, 2019

/retest

@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 20, 2019

@cgwalters Tests did pass twice ( https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/openshift_machine-config-operator/542/pull-ci-openshift-machine-config-operator-master-e2e-aws/2628/ after this week’s rebase.

AFAICS most of the failures in https://openshift-gce-devel.appspot.com/pr/openshift_machine-config-operator/542 are caused by either AWS RequestLimitExceeded errors, or by e2e tests failing — but those tests running at all means that the pause container is usable, doesn’t it?

(Warning: I still have very little idea what I’m doing.)

@mtrmac
Copy link
Contributor Author

mtrmac commented Mar 20, 2019

Tests did pass twice ( https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/openshift_machine-config-operator/542/pull-ci-openshift-machine-config-operator-master-e2e-aws/2628/

… and https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/openshift_machine-config-operator/542/pull-ci-openshift-machine-config-operator-master-e2e-aws/2680/ )

@cgwalters cgwalters mentioned this pull request Mar 22, 2019
Merged
@kikisdeliveryservice
Copy link
Contributor

kikisdeliveryservice commented Mar 22, 2019

#540 has been approved so closing this PR since it's not needed anymore. Thanks!

@mtrmac mtrmac deleted the pause_image_auth_file-tests branch March 22, 2019 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mtrmac @openshift-ci-robot @cgwalters @kikisdeliveryservice