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.

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

package_version check: stop looking for docker #7347

Merged

Conversation

@sosiouxme
Copy link
Member

commented Mar 1, 2018

Removes the part of the check that ensures the available Docker version
matches the OpenShift version to be installed, because we can't really
determine the available Docker version.

At the time that this check runs, the excluders have likely not been
run. With the release of docker-1.13, versions prior to OpenShift 3.9
will not be compatible, and we will be relying on the docker excluder to
ensure openshift-ansible does not install it. But as the check looks at
the state of yum to see what would be installed, and the exclusion is
not in place, the check cannot accurately determine if the docker-1.13
it sees is going to be installed, or if something compatible will be.
Rather than give a bogus error when 1.13 is released, or add some
horrible hack, just stop looking at docker.

package_version check: stop looking for docker
Removes the part of the check that ensures the available Docker version
matches the OpenShift version to be installed, because we can't really
determine the available Docker version.

At the time that this check runs, the excluders have likely not been
run. With the release of docker-1.13, versions prior to OpenShift 3.9
will not be compatible, and we will be relying on the docker excluder to
ensure openshift-ansible does not install it. But as the check looks at
the state of yum to see what would be installed, and the exclusion is
not in place, the check cannot accurately determine if the docker-1.13
it sees is going to be installed, or if something compatible will be.
Rather than give a bogus error when 1.13 is released, or add some
horrible hack, just stop looking at docker.
@michaelgugino
Copy link
Contributor

left a comment

I approve. Perhaps we should let some other comment before we remove this check.

@mtnbikenc @sdodson thoughts?

@sdodson

This comment has been minimized.

Copy link
Member

commented Mar 1, 2018

I'm fine with this.
/lgtm

@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2018

I think those all had to do with the skopeo dependency being broken, although MODULE ERROR isn't too specific and should do something more robust.

/retest

@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 2, 2018

Looks like the version of skopeo in dockertested lacks a corresponding skopeo-containers. Tests aren't getting very far. @stevekuznetsov

@stevekuznetsov

This comment has been minimized.

Copy link
Contributor

commented Mar 2, 2018

We pull in whatever the current level of packages as they are tagged in the latest 7.4 candidate tag in Brew. Last time this was:

docker-1.13.1-53.git774336d.el7
container-selinux-2.42-1.gitad8f0f7.el7
container-storage-setup-0.8.0-3.git1d27ecf.el7
skopeo-0.1.28-1.git0270e56.el7
atomic-1.22.1-1.gitd36c015.el7
python-pytoml-0.1.14-1.git7dea353.el7
oci-register-machine-0-6.git2b44233.el7

Apparently skopeo-containers is subsumed by skopeo now? openshift/aos-cd-jobs@4edcb2e

@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 4, 2018

@stevekuznetsov looking in brew, there's still a skopeo-containers subpackage being built. And it's being included in dockertested; I was wrong, the error message from here indicates something else is actually requiring a different version of skopeo:

--> Processing Dependency: libnet.so.1()(64bit) for package: criu-2.12-2.el7.x86_64
...
---> Package skopeo.x86_64 1:0.1.27-3.dev.git14245f2.el7 will be installed
--> Processing Dependency: skopeo-containers = 1:0.1.27-3.dev.git14245f2.el7 for package: 1:skopeo-0.1.27-3.dev.git14245f2.el7.x86_64
--> Running transaction check
...
---> Package skopeo.x86_64 1:0.1.27-3.dev.git14245f2.el7 will be installed
--> Processing Dependency: skopeo-containers = 1:0.1.27-3.dev.git14245f2.el7 for package: 1:skopeo-0.1.27-3.dev.git14245f2.el7.x86_64
--> Finished Dependency Resolution
Error: Package: 1:skopeo-0.1.27-3.dev.git14245f2.el7.x86_64 (oso-rhui-rhel-server-extras)
           Requires: skopeo-containers = 1:0.1.27-3.dev.git14245f2.el7
           Installed: 1:skopeo-containers-0.1.28-1.git0270e56.el7.x86_64 (@httpsmirroropenshiftcomenterpriserheldockertestedx8664os)
               skopeo-containers = 1:0.1.28-1.git0270e56.el7

I'm not quite clear how it's coming up with skopeo-0.1.27-3.dev.git14245f2.el7.x86_64 -- that doesn't seem right.

@stevekuznetsov

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2018

If we installed docker from dockertested and it pulled in skopeo-containers at a high level, then we turned off the dockertested repo and someone tries to install skopeo from RHUI it will attempt to install it's required version of skopeo-containers but sees that a higher level is already present. This is why we had turned on the dockertested repo at all times. Now we only turn it on with the latest branches, I think. Can you check in your jobs if this failure mode is what you're seeing?

@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 6, 2018

That doesn't sound like what I saw as it seemed both skopeo and skopeo-containers were already installed, and then something wanted an earlier (dev) version of skopeo. Perhaps there was a change in the provides such that only the earlier version fits the requirements -- I don't think anything says "give me this specific version of skopeo", and anything that just tried to install skopeo without qualification would find it's already there.

But what the heck, let's see how things are now...
/retest

@openshift-ci-robot

This comment has been minimized.

Copy link

commented Mar 6, 2018

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

Test name Commit Details Rerun command
ci/openshift-jenkins/logging 3cec52c link /test logging
ci/openshift-jenkins/system-containers 3cec52c link /test system-containers
ci/openshift-jenkins/extended_conformance_install_crio 3cec52c link /test crio
ci/openshift-jenkins/containerized 3cec52c link /test containerized
ci/openshift-jenkins/install 3cec52c link /test install

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.

@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 6, 2018

@stevekuznetsov still failing much the same.
If I'm reading https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/openshift_openshift-ansible/7347/test_pull_request_openshift_ansible_extended_conformance_install_crio/3085/ correctly, skopeo-containers-0.1.28-1.git0270e56.el7.x86_64 is already installed from dockertested, but not skopeo itself. Now that something needs skopeo, that's getting skopeo-0.1.27-3.dev.git14245f2.el7.x86_64 from oso-rhui-rhel-server-extras and yum refuses to downgrade the installed skopeo-containers. So... would be solved if corresponding skopeo were installed at the same time as skopeo-containers.

BTW this is in STAGE: INSTALL SKOPEO before even running ansible.

@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 8, 2018

@stevekuznetsov this just got a good bit more urgent since docker-1.13 was released (did not know it would be so soon). Suddenly everyone will be hitting https://bugzilla.redhat.com/show_bug.cgi?id=1551862 on GA installs/upgrades.

Can we fix CI or @sdodson is this a candidate for the green button? Thing is, tests fail before they've really tested anything.

@sdodson

This comment has been minimized.

Copy link
Member

commented Mar 8, 2018

@sosiouxme Yeah, there's bugs for this so QE will be validating the fix too. Merging.

@sdodson sdodson merged commit 10251ac into openshift:release-3.7 Mar 8, 2018

4 of 10 checks passed

ci/openshift-jenkins/containerized Jenkins job failed.
Details
ci/openshift-jenkins/extended_conformance_install_crio Jenkins job failed.
Details
ci/openshift-jenkins/install Jenkins job failed.
Details
ci/openshift-jenkins/logging Jenkins job failed.
Details
ci/openshift-jenkins/system-containers Jenkins job failed.
Details
Submit Queue Required Github CI test is not green: ci/openshift-jenkins/install
Details
ci/openshift-jenkins/gcp Skipped
ci/openshift-jenkins/tox Jenkins job succeeded.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fedora/27/atomic All tests passed.
Details
@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 8, 2018

/cherrypick release-3.6

This doesn't appear to have checked for specific versions before that.

@openshift-cherrypick-robot

This comment has been minimized.

Copy link

commented Mar 8, 2018

@sosiouxme: new pull request created: #7456

In response to this:

/cherrypick release-3.6

This doesn't appear to have checked for specific versions before that.

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.

@sosiouxme sosiouxme deleted the sosiouxme:20180301-pkg-version-docker-3.7 branch Mar 8, 2018

@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 8, 2018

@liggitt

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2018

does this need picking to release-3.8 to turn openshift/origin#18955 (comment) green?

@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2018

@liggitt if origin release-3.8 tests use openshift-ansible 3.8, then yes. I'm not sure that's the case though; if o-a 3.9 is in use, it hasn't been updated to allow docker-1.13 for 3.8, just 3.9. Is 3.8 supposed to work on docker-1.13?

@michaelgugino

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2018

@liggitt @sosiouxme I don't believe we are planning to ship openshift-ansible for release-3.8; The 3.8 branch has drifted significantly; we should be able to utilize openshift-ansible 3.9 for any origin 3.8 related testing.

@sdodson

This comment has been minimized.

Copy link
Member

commented Mar 22, 2018

does this need picking to release-3.8 to turn openshift/origin#18955 (comment) green?

It looks like CI jobs like https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/19042/test_pull_request_origin_extended_conformance_gce_37/5/ which targets release-3.7 was using openshift/origin-ansible:v3.9 and https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/18955/test_pull_request_origin_extended_conformance_gce/17693/ which was against release-3.8 used openshift/origin-ansible:latest

I think forward porting this change to master, release-3.8, and release-3.9 will make those problems go away but we should figure why the GCP job is using the wrong images.

@smarterclayton

@smarterclayton

This comment has been minimized.

Copy link
Member

commented Mar 22, 2018

@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 22, 2018

let's see what happens:

/cherry-pick release-3.8
/cherry-pick release-3.9
/cherry-pick master

@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 22, 2018

/cherrypick release-3.8
/cherrypick release-3.9
/cherrypick master

@openshift-cherrypick-robot

This comment has been minimized.

Copy link

commented Mar 22, 2018

@sosiouxme: #7347 failed to apply on top of branch "release-3.8":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	roles/openshift_health_checker/openshift_checks/package_version.py
M	roles/openshift_health_checker/test/package_version_test.py
Falling back to patching base and 3-way merge...
Auto-merging roles/openshift_health_checker/test/package_version_test.py
CONFLICT (content): Merge conflict in roles/openshift_health_checker/test/package_version_test.py
Auto-merging roles/openshift_health_checker/openshift_checks/package_version.py
CONFLICT (content): Merge conflict in roles/openshift_health_checker/openshift_checks/package_version.py
Patch failed at 0001 package_version check: stop looking for docker

In response to this:

/cherrypick release-3.8
/cherrypick release-3.9
/cherrypick master

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.

@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 23, 2018

/cherrypick release-3.9

@openshift-cherrypick-robot

This comment has been minimized.

Copy link

commented Mar 23, 2018

@sosiouxme: #7347 failed to apply on top of branch "release-3.9":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	roles/openshift_health_checker/openshift_checks/package_version.py
M	roles/openshift_health_checker/test/package_version_test.py
Falling back to patching base and 3-way merge...
Auto-merging roles/openshift_health_checker/test/package_version_test.py
CONFLICT (content): Merge conflict in roles/openshift_health_checker/test/package_version_test.py
Auto-merging roles/openshift_health_checker/openshift_checks/package_version.py
CONFLICT (content): Merge conflict in roles/openshift_health_checker/openshift_checks/package_version.py
Patch failed at 0001 package_version check: stop looking for docker

In response to this:

/cherrypick release-3.9

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.

@sosiouxme

This comment has been minimized.

Copy link
Member Author

commented Mar 23, 2018

meh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.