#68 Invalid data returned
Closed 7 years ago Opened 7 years ago by pingou.

Running the following script:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
#!/usr/bin/env python
import requests
import json
import pprint

nvr = 'python-yubico-1.3.2-7.fc26'
update_id = 'FEDORA-2017-63975ea815'

url = 'https://greenwave-web-greenwave.app.os.fedoraproject.org/api/v1.0/decision'

data = dict(
    # https://pagure.io/greenwave/blob/master/f/conf/policies/fedora.yaml
    decision_context='bodhi_update_push_stable',
    product_version='fedora-26',
    subject=[
        {'item': update_id, 'type': 'bodhi_update'},
        {'item': nvr, 'type': 'koji_build'},
        {'original_spec_nvr': nvr},
    ],
)
print("Querying %s with:" % url)
pprint.pprint(data)
response = requests.post(url, json=data)
print("Got %r" % response)
pprint.pprint(response.json())

leads to surprising results

So this is regarding the update https://bodhi.fedoraproject.org/updates/FEDORA-2017-63975ea815 for which the UI shows all tests passing.

The summary of the decision is 'summary': '8 of 12 required tests not found', but it seems to want to get taskotron results for original_spec_nvr and bodhi_update items and this is not going to work :(


@pingou - we fixed it in https://koji.fedoraproject.org/koji/taskinfo?taskID=21450878 you were missing an important patch from #63.

That's included now and the response from greenwave looks sane.

~❯ cat testgreenwave.py 
#!/usr/bin/env python
import requests
import json
import pprint

nvr = 'python-yubico-1.3.2-7.fc26'
update_id = 'FEDORA-2017-63975ea815'

url = 'https://greenwave-web-greenwave.app.os.fedoraproject.org/api/v1.0/decision'

data = dict(
    # https://pagure.io/greenwave/blob/master/f/conf/policies/fedora.yaml
    decision_context='bodhi_update_push_stable',
    product_version='fedora-26',
    subject=[
        {'item': update_id, 'type': 'bodhi_update'},
        {'item': nvr, 'type': 'koji_build'},
        {'original_spec_nvr': nvr},
    ],
)
print("Querying %s with:" % url)
pprint.pprint(data)
response = requests.post(url, json=data)
print("Got %r" % response)
pprint.pprint(response.json())
~❯ python testgreenwave.py 
Querying https://greenwave-web-greenwave.app.os.fedoraproject.org/api/v1.0/decision with:
{'decision_context': 'bodhi_update_push_stable',
 'product_version': 'fedora-26',
 'subject': [{'item': 'FEDORA-2017-63975ea815', 'type': 'bodhi_update'},
             {'item': 'python-yubico-1.3.2-7.fc26', 'type': 'koji_build'},
             {'original_spec_nvr': 'python-yubico-1.3.2-7.fc26'}]}
Got <Response [200]>
{u'applicable_policies': [u'taskotron_release_critical_tasks',
                          u'atomic_ci_pipeline_results'],
 u'policies_satisified': False,
 u'summary': u'2 of 4 required tests not found',
 u'unsatisfied_requirements': [{u'item': {u'item': u'python-yubico-1.3.2-7.fc26',
                                          u'type': u'koji_build'},
                                u'testcase': u'dist.abicheck',
                                u'type': u'test-result-missing'},
                               {u'item': {u'item': u'python-yubico-1.3.2-7.fc26',
                                          u'type': u'koji_build'},
                                u'testcase': u'dist.upgradepath',
                                u'type': u'test-result-missing'}]}

I have had an interesting discussion with @kparal about the two missing tests here:

  • kparal | pingou: currently we run abicheck just for critpath packages. and even some of them are blacklisted due to us running to OOM issues. I think we could easily extend the coverage to all packages not just critpath, we just need to stay more vigilant in maintaining the OOM blacklist. e.g. libreoffice can't be easily processed with abicheck with sane amounts of RAM
  • as for upgradepath, we currently execute it only when the maintainer submits the update for stable. there is a convoluted reasoning regarding the fact, that we didn't want to display the results as FAILED for an update that's available for F25+F26+F27 in Bodhi (F26 would be marked as failed until F27 was pushed, then F25 would be marked as failed until F26 was pushed). so we wait until the person submits the updates stable (usually all of them). then we can be quite certain all of them would be pushed at the same time and give more reliable and non-confusing results. it's historical and related to the manual submission process. I'm sure we could redo it in a better way when we consider it a gating task, instead of an information task for a person doing a manual process. So, right now, the package needs to be moved to fxx-updates-pending in order to be tested with upgradepath

So we may want to tweak the config accordingly.

I think we should:

  • Disable upgradepath check on pending testing updates for now (and work with @kparal on how to reintroduce it)
  • Disable abicheck or combine it with a blacklist of packages that are not able of being run through it (which is also what @kparal was suggesting although he was more whitelist but that would mean updating the list everytime a package is added to Fedora :-s)

I think we're also missing something on the bodhi front since it always sends a decision_context': u'bodhi_update_push_stable' while it actually queries for updates pending stable and pending testing, we may have to adjust this.

More about upgradepath:
It would be best for this test to run greenwave on updates-pending content. It can't give reliable results for updates-testing packages, because it might happen that fc25 update is submitted (either manually or using autokarma) before fc26 update, and then upgradepath would be broken. Or we can stop about upgradepath completely (but that would be a question for FESCo).

Disable abicheck or combine it with a blacklist of packages that are not able of being run through it (which is also what @kparal was suggesting although he was more whitelist but that would mean updating the list everytime a package is added to Fedora :-s)

I meant blacklist. The number of packages that can't be checked with abicheck with usual amount of RAM should not be that high - tens of packages at most, I think.

Okay this is good to know. I guess in the short term we will need to just take out dist.upgradepath and dist.abicheck from the policy, until we find some solutions to the above.

So as I understand, what we need to do here are:

  • take out dist.upgradepath from the policy for pushing update to stable.
  • A new policy for pushing update to pending which will require dist.upgradepath as well as dist.abicheck and dist.rpmdeplint
  • A new policy for ignoring the test results of dist.abicheck of the blacklist packages or maybe we could somehow expand the current policy to support this?

For the blacklist, not sure this is the best approach but at least we get a starting point, see PR#91.
@kparal, could you please give the list of the packages that can't be checked with abicheck so I can use it to write a test?

@mjia The exclude list is kept here:
https://infrastructure.fedoraproject.org/cgit/ansible.git/tree/roles/taskotron/taskotron-trigger/templates/trigger_rules.yml.j2#n17 (line 17)
The current value is:

[firefox,thunderbird,kernel,kdelibs,kdepim,qt,mariadb,java-1.8.0-openjdk,libreoffice]

@kparal - at some point, can we move that to an ansible var?

That way, we can maintain the exception list in one place.

Both the taskotron and greenwave ansible templates can then use the var to write out their exceptions.

@ralph Sure, that sounds reasonable. If you know where to put this to that it can be shared by both ansible roles, please go ahead. I'm not completely sure how ansible vars sharing works in there.

OK, I moved this to a var in these two commits:

I verified that it produced the same output on the taskotron target systems by running the playbook with --check --diff.

Metadata Update from @ralph:
- Issue status updated to: Closed (was: Open)

7 years ago

Weird, now that I look, it seems like abicheck is not run all that frequently.

Here's an example query to greenwave that now fails due to the abicheck requirement. Maybe I got this wrong.

{u'applicable_policies': [u'taskotron_release_critical_tasks_for_stable',
                          u'taskotron_release_critical_tasks_for_stable_with_blacklist'],
 u'policies_satisified': False,
 u'summary': u'1 of 3 required tests not found',
 u'unsatisfied_requirements': [{u'item': {u'item': u'kf5-kded-5.39.0-1.fc27',
                                          u'type': u'koji_build'},
                                u'testcase': u'dist.abicheck',
                                u'type': u'test-result-missing'}]}

I think the problem is we are missing this commit

https://pagure.io/greenwave/c/f00076f9b159d42d3554ed41bfa65cb9a1b858bc?branch=master

I will make a new build and update the stagging.

I'm going to re-open this. I didn't realize it was also whitelisted to only the critpath packages.

Metadata Update from @ralph:
- Issue status updated to: Open (was: Closed)

7 years ago

I think we could easily extend the coverage to all packages not just critpath, we just need to stay more vigilant in maintaining the OOM blacklist. e.g. libreoffice can't be easily processed with abicheck with sane amounts of RAM

@kparal, can we do this ^^?

I think the problem is we are missing this commit
https://pagure.io/greenwave/c/f00076f9b159d42d3554ed41bfa65cb9a1b858bc?branch=master
I will make a new build and update the stagging.

Actually, it is not easy to make a new build with this commit due to some conflicts, so this would go to 0.4.

Metadata Update from @ralph:
- Issue status updated to: Closed (was: Open)

7 years ago

Log in to comment on this ticket.

Metadata