Project

Profile

Help

Issue #2134

Updating a repo without specifying checksum_type causes KeyError

Added by mhrivnak over 4 years ago. Updated almost 2 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
High
Assignee:
Sprint/Milestone:
-
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version:
2.8.4
Platform Release:
2.9.3
OS:
Triaged:
No
Groomed:
No
Sprint Candidate:
No
Tags:
Easy Fix, Pulp 2
Sprint:
Quarter:

Description

From bugzilla:

On updating a repo without touching checksum_type attribute, pulp raises an unexpected KeyError:

Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352) Task pulp.server.tasks.repository.distributor_update[36edf787-70ca-437a-b840-a3aebba3fd4e] raised unexpected: KeyError(u'checksum_type',)
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352) Traceback (most recent call last):
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352)   File "/usr/lib/python2.7/site-packages/celery/app/trace.py", line 240, in trace_task
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352)     R = retval = fun(*args, **kwargs)
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352)   File "/usr/lib/python2.7/site-packages/pulp/server/async/tasks.py", line 473, in __call__
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352)     return super(Task, self).__call__(*args, **kwargs)
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352)   File "/usr/lib/python2.7/site-packages/pulp/server/async/tasks.py", line 103, in __call__
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352)     return super(PulpTask, self).__call__(*args, **kwargs)
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352)   File "/usr/lib/python2.7/site-packages/celery/app/trace.py", line 437, in __protected_call__
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352)     return self.run(*args, **kwargs)
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352)   File "/usr/lib/python2.7/site-packages/pulp/server/controllers/distributor.py", line 211, in update
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352)     distributor.config.pop(k)
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352)   File "/usr/lib/python2.7/site-packages/mongoengine/base/datastructures.py", line 73, in pop
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352)     return super(BaseDict, self).pop(*args, **kwargs)
Jul 15 10:50:32 rhsm-qe-3 pulp: celery.worker.job:ERROR: (4746-12352) KeyError: u'checksum_type'

Things seem to go wrong here: https://github.com/pulp/pulp/blob/2.8-dev/server/pulp/server/controllers/distributor.py#L211


Related issues

Has duplicate Pulp - Issue #2076: distributor update fails trying to remove already-missing keyCLOSED - DUPLICATE<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>

Associated revisions

Revision a4038809 View on GitHub
Added by fdobrovo over 4 years ago

Fix crash from removing missing option from dist conf

Fixed crash that arise when we try to remove an option from distributor confing which was not before set. This ends up in KeyError because this option was not in config dict. This was fixed and also added TestCase for it.

Summary:

  • Fixed pulp.server.controllers.distributor.update to ignore missing options from distributor config
  • Added TestCase for this behaviour TestUpdate.test_remove_none_values_non_existing()

closes #2134 https://pulp.plan.io/issues/2134

Revision a4038809 View on GitHub
Added by fdobrovo over 4 years ago

Fix crash from removing missing option from dist conf

Fixed crash that arise when we try to remove an option from distributor confing which was not before set. This ends up in KeyError because this option was not in config dict. This was fixed and also added TestCase for it.

Summary:

  • Fixed pulp.server.controllers.distributor.update to ignore missing options from distributor config
  • Added TestCase for this behaviour TestUpdate.test_remove_none_values_non_existing()

closes #2134 https://pulp.plan.io/issues/2134

History

#1 Updated by fdobrovo over 4 years ago

  • Assignee set to fdobrovo

#2 Updated by ipanova@redhat.com over 4 years ago

  • Status changed from NEW to ASSIGNED

#3 Updated by amacdona@redhat.com over 4 years ago

Did not triage because we are looking for more info on replication.

#4 Updated by dalley over 4 years ago

Granted, there is no information here as to what command, specifically, caused the error for this user, but I have found a way to reproduce it this error message.

pulp-admin rpm repo update --repo-id zoo --checksum-type=

The above code will cause the same error if "checksum_type" is not previously defined in the distributor config.

I'm not sufficiently familiar with Pulp's CLI to know if that's supposed to be valid syntax or not, but in any case, I believe that is what the user is doing and what they meant by "not specifying the 'checksum_type'".

The root cause is that the config being passed into update(..., config=None, ...) contains keys not included in the distributor config. Thus when the code does this:

for k, v in config.iteritems():
    if v is None:
        distributor.config.pop(k)
    else:
distributor.config[k] = v

"v" evaluates to None, and the key "k" is not already present in the distributor config, and so it will error when it attempts to "pop" the key off.

#5 Updated by fdobrovo over 4 years ago

  • Tags Easy Fix added

If that so the only thing needed is to convert.

distributor.config.pop(k) -> distributor.config.pop(k, None)

#6 Updated by fdobrovo over 4 years ago

  • Status changed from ASSIGNED to POST

The title should more likely be "Removing option from distributor config which was not set before causes KeyError"

#8 Updated by fdobrovo over 4 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100

#9 Updated by fdobrovo over 4 years ago

  • Platform Release set to 2.9.3

#10 Updated by semyers over 4 years ago

  • Status changed from MODIFIED to 5

#11 Updated by elyezer over 4 years ago

  • Status changed from 5 to 6

I have verified this on a Pulp 2.9.3b1 installation.

Step to verify:

1 pulp-admin login -u admin -p admin
2 pulp-admin rpm repo create --repo-id zoo
3 pulp-admin rpm repo update --repo-id zoo --checksum-type= # update with a null checksum-type
4 pulp-admin rpm repo list --repo-id zoo --details
5 pulp-admin rpm repo update --repo-id zoo --checksum-type=sha256 # update to a valid checksum-type
6 pulp-admin rpm repo list --repo-id zoo --details
7 pulp-admin rpm repo update --repo-id zoo --checksum-type= # update back to a null checksum-type
8 pulp-admin rpm repo list --repo-id zoo --details

#12 Updated by elyezer over 4 years ago

An automated test was written to cover this issue [1]. That will ensure that a regression does not happen.

[1] https://github.com/PulpQE/pulp-smash/pull/389

#13 Updated by semyers over 4 years ago

  • Status changed from 6 to CLOSED - CURRENTRELEASE

#14 Updated by ipanova@redhat.com over 4 years ago

  • Has duplicate Issue #2076: distributor update fails trying to remove already-missing key added

#17 Updated by bmbouter almost 2 years ago

  • Tags Pulp 2 added

Please register to edit this issue

Also available in: Atom PDF