Rockon Container Links cannot handle duplicate sources

[Hooverdan96]

As observed by @phillxnet during his experiments on a BareOS Rockon Draft PR 383 associated with this Issue: BareOS Rockon #94, I have experimented with a simplified linked Rocknet (aka docker network setup) to replicate the observed issue w.r.t. duplicate keys during the Rockon updates.

I created two definitions containing 1:n (RocknetTest.json) and n:1 (RocknetTest2.json) container links, respectively.

RockNet_Test.json
RockNet_Test2.json

the 1:n link is represented like this:

        "container_links": {
            "source_container_1": [
                {
                    "name": "source_to_target1",
                    "source_container": "target_container_1"
                },
                {
                    "name": "source_to_target2",
                    "source_container": "target_container_2"
                }
            ]

n:1 like this:

        "container_links": {
            "2_target_container_1": [
                {
                    "name": "2_target1_to_source",
                    "source_container": "2_source_container_1"
                }
            ],
            "2_target_container_2": [
                {
                    "name": "2_target2_to_source",
                    "source_container": "2_source_container_1"
                }
            ]

The first json definition updated fine into the Rockon database tables (and also provided a successful installation), the second one failed with the following error:

Houston, we've had a problem.
Errors occurred while processing updates for the following Rock-ons (RockNet_Test_2: duplicate key value violates unique constraint "storageadmin_dcontainerlink_source_id_key" DETAIL: Key (source_id)=(93) already exists.).

This is the same symptom that @phillxnet encountered.

One root cause could be that during the Django upgrade the format of the constraint meta tag unique_together changed from tuple of tuple (in 2.x) to list of lists (4.x). If that's the case, the code should be inspected for other this tag in other meta definitions and probably adjusted at the same time. With the above information it should be possible to test out this theory.

[phillxnet]

Linking to @Hooverdan96 comments re suspected causes for this unintended and unexpected limitation re Django updates in our current testing channel:

• BareOS server set #94 rockon-registry#383 (comment)

[phillxnet]

Noting that we need to establish if this bug is also exibited by our last Stable release. Where the model unique_together constraint was proposed to be appropriate. It has only been in this current testing phase that we have updated our Django to supposedly cause this issue: by not also adapting that clause in the model.

[phillxnet]

The draft PR's Rock-on (Bareos backup server), in reproducer state for this issue, i.e. duplicated "source_container" entries (across two networks in that case) also reproduced this error when added via the local /opt/rockstor/rockons-metastore/ file repo on an instance of our last Stable release (v4.6.1-0) with a base OS of 15.5 (via our in-place update from 15.4 to 15.5):

[09/Dec/2024 13:01:16] ERROR [storageadmin.util:45] Exception: Errors occurred while processing updates for the following Rock-ons (Bareos-backup-server: duplicate key value violates unique constraint "storageadmin_dcontainerlink_source_id_key"
DETAIL:  Key (source_id)=(95) already exists.
).
Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/storageadmin/views/rockon.py", line 139, in post
    self._create_update_meta(r, rockons[r])
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/utils/decorators.py", line 185, in inner
    return func(*args, **kwargs)
  File "/opt/rockstor/src/rockstor/storageadmin/views/rockon.py", line 414, in _create_update_meta
    source=sco, destination=co
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/models/query.py", line 466, in get_or_create
    return self._create_object_from_params(lookup, params)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/models/query.py", line 509, in _create_object_from_params
    six.reraise(*exc_info)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/models/query.py", line 500, in _create_object_from_params
    obj = self.create(**params)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/models/query.py", line 394, in create
    obj.save(force_insert=True, using=self.db)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/models/base.py", line 808, in save
    force_update=force_update, update_fields=update_fields)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/models/base.py", line 838, in save_base
    updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/models/base.py", line 924, in _save_table
    result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/models/base.py", line 963, in _do_insert
    using=using, raw=raw)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/models/query.py", line 1079, in _insert
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 1112, in execute_sql
    cursor.execute(sql, params)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/opt/rockstor/.venv/lib/python2.7/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
IntegrityError: duplicate key value violates unique constraint "storageadmin_dcontainerlink_source_id_key"
DETAIL:  Key (source_id)=(95) already exists.

I.e. when we were using Python 2.7 and Django 1.11.29:

installer:/opt/rockstor # poetry show | grep "^django "
django               1.11.29   A high-level Python Web framework that encourages rapid development and clean, pragmatic design.

[phillxnet]

From storageadmin_dcontainerlink_source_id_key I think our one-to-one setup is the limitation here. The same excerpt from our model as noted by @Hooverdan96 in the already referenced initial reproducer (from master branch as unchanged in testing):

rockstor-core/src/rockstor/storageadmin/models/rockon.py

Lines 113 to 120 in 4d60a2c

	class DContainerLink(models.Model):
	source = models.OneToOneField(DContainer)
	destination = models.ForeignKey(DContainer, related_name="destination_container")
	name = models.CharField(max_length=64, null=True)
	class Meta:
	unique_together = ("source", "destination", "name")
	app_label = "storageadmin"

I.e. only one object at each end of a dcontainerlink.source <-> dcontainer.

Which I read as each dcontainer can only have a single relationship to a sole dcontainerlink.source.

@FroggyFlox Is that your reading of this constraint?

[FroggyFlox]

@phillxnet... It appears you are correct, which means I used the wrong relationship there to being with 😞. I'm sorry for my mistake there; that was a a blatant misunderstanding of Django's OneToOne field. It shows I should be better at actually testing things instead of assuming I understand them properly.

[phillxnet]

@FroggyFlox Thanks for the confirmation on my model suspicions here re the OneToOne thing. No worries at all though: we just need to fathom a way through what we currently have in pending Stable (v5.1.x-x) and last Stable (v4.6.1.0). As I'd hoped, in the Bareos backup server, to minimise user intervention on getting a working docker-network. We could potentially develop a fix for both last and pending stable (current testing) but I'm keen to not break last stable via a Rock-ons definition. And our last stable was released for 15.4. Even given it can sustain our 15.4 to 15.5 treatment.

In case we have both miss-interpreted the OneToOneField() there is the following examples orientated doc entry from Django's excellent documentation:

https://docs.djangoproject.com/en/4.2/topics/db/examples/one_to_one/

---

[EDIT] We also have the OneToOne field definition as per our current (testing) Django docs entry:

https://docs.djangoproject.com/en/4.2/ref/models/fields/#django.db.models.OneToOneField

A one-to-one relationship. Conceptually, this is similar to a ForeignKey with unique=True, but the “reverse” side of the relation will directly return a single object.

Given the depth of this definition in our Rock-ons: I'm strongly favouring defining this as a newly surfaced limitation of our current Rock-ons "container_links" element. I.e. it was, after all, mutated from what came before re --link container that was rug-pulled form under us by upstream docker and @FroggyFlox saved the day for all existing Rock-ons definitions out there by re-purposing that element to work via docker networks instead. Any limitation we see here now, could alternatively be resolved by implementing a new Rock-on element that pertains directly to docker networks and is implemented using a new updated set of classes. Where we then remove this limitation. That would likely be safer and more flexible going forward than to mess with what we have now had for a few years. And this limitation has only just surfaced.

[phillxnet]

@Hooverdan96 & @FroggyFlox I'm tempted to kick this down the road for now, and address via a new json element that was actually proposed by @FroggyFlox originally: regarding a more full support of docker networks directly. And we then deprecate the old "container_links" Rock-on element. We then get to re-define how that works and this issue is the start of our documentation on the limitations we looks to have build-in here re our inheritance of the concepts associated with the --link approach we were coming from.