Makefile: Update Cockpit lib to 79623a0a425600da302f66a2752afe35

[github-actions]

Requirements/split out:

• Fix magic env var value auto-splitting #1833
• Some test fixes #1834

[jelly]

The issue is the click in Firefox now clicks in the middle of the CPU Shares NumberInput which doesn't trigger the checkbox to get checked. This is a bug in PF, so we work around here by clicking on the left button.

Maybe a better solution is:

        b.mouse("#folder-view tbody", "click", x=1, y=1)

[jelly]

Locally I get:

INFO:bidi.command:← script.evaluate({'expression': 'ph_wait_cond(() => ph_is_visible("#containers-containers #table-testpod1 tbody tr[data-row-id=\\"e26a27f009ab22367af6bfcb07182034b6387edc50e4488dc53d3028cfec6961true\\"] td[data-label=State]"), 15000, null)', 'awaitPromise': True, 'target': {'context': '6442450947'}}) [id 276]
INFO:bidi.command:→ {'realm': '00cea0c4-a123-4c6a-85a9-bdaf6e21fa12', 'type': 'success', 'result': None}
INFO:bidi.command:← script.evaluate({'expression': 'ph_text("#containers-containers #table-testpod1 tbody tr[data-row-id=\\"e26a27f009ab22367af6bfcb07182034b6387edc50e4488dc53d3028cfec6961true\\"] td[data-label=State]")', 'awaitPromise': True, 'target': {'context': '6442450947'}}) [id 277]
INFO:bidi.command:→ {'realm': '00cea0c4-a123-4c6a-85a9-bdaf6e21fa12', 'type': 'success', 'result': 'Running'}
INFO:bidi.command:← script.evaluate({'expression': 'ph_wait_cond(() => ph_is_present("#containers-containers tbody tr:contains(\'test-pod-1-system\') .pf-v5-c-table__toggle button:not([disabled]):not([aria-disabled=true])"), 15000, null)', 'awaitPromise': True, 'target': {'context': '6442450947'}}) [id 278]
INFO:bidi.command:→ {'realm': '00cea0c4-a123-4c6a-85a9-bdaf6e21fa12', 'type': 'success', 'result': None}
INFO:bidi.command:← script.evaluate({'expression': 'ph_wait_cond(() => ph_is_visible("#containers-containers tbody tr:contains(\'test-pod-1-system\') .pf-v5-c-table__toggle button:not([disabled]):not([aria-disabled=true])"), 15000, null)', 'awaitPromise': True, 'target': {'context': '6442450947'}}) [id 279]
INFO:bidi.command:→ {'realm': '00cea0c4-a123-4c6a-85a9-bdaf6e21fa12', 'type': 'success', 'result': None}
INFO:bidi.command:← script.evaluate({'expression': 'ph_find_scroll_into_view("#containers-containers tbody tr:contains(\'test-pod-1-system\') .pf-v5-c-table__toggle button:not([disabled]):not([aria-disabled=true])")', 'awaitPromise': True, 'target': {'context': '6442450947'}}) [id 280]
INFO:bidi.command:→ {'realm': '00cea0c4-a123-4c6a-85a9-bdaf6e21fa12', 'type': 'success', 'result': {'type': 'node', 'sharedId': '20e3b103-7c5f-41d5-aada-50b85c57d81b', 'value': {'nodeType': 1, 'localName': 'button', 'namespaceURI': 'http://www.w3.org/1999/xhtml', 'childNodeCount': 1, 'attributes': {'aria-labelledby': 'simple-nodee26a27f009ab22367af6bfcb07182034b6387edc50e4488dc53d3028cfec6961true expand-togglee26a27f009ab22367af6bfcb07182034b6387edc50e4488dc53d3028cfec6961true', 'id': 'expand-togglee26a27f009ab22367af6bfcb07182034b6387edc50e4488dc53d3028cfec6961true', 'aria-expanded': 'false', 'aria-disabled': 'false', 'aria-label': 'Details', 'class': 'pf-v5-c-button pf-m-plain', 'type': 'button', 'data-ouia-component-type': 'PF5/Button', 'data-ouia-safe': 'true', 'data-ouia-component-id': 'OUIA-Generated-Button-plain-2'}, 'shadowRoot': None}}}
INFO:bidi.command:← input.performActions({'context': '6442450947', 'actions': [{'id': 'pointer-281', 'type': 'pointer', 'parameters': {'pointerType': 'mouse'}, 'actions': [{'type': 'pointerMove', 'x': 0, 'y': 0, 'origin': {'type': 'element', 'element': {'type': 'node', 'sharedId': '20e3b103-7c5f-41d5-aada-50b85c57d81b', 'value': {'nodeType': 1, 'localName': 'button', 'namespaceURI': 'http://www.w3.org/1999/xhtml', 'childNodeCount': 1, 'attributes': {'aria-labelledby': 'simple-nodee26a27f009ab22367af6bfcb07182034b6387edc50e4488dc53d3028cfec6961true expand-togglee26a27f009ab22367af6bfcb07182034b6387edc50e4488dc53d3028cfec6961true', 'id': 'expand-togglee26a27f009ab22367af6bfcb07182034b6387edc50e4488dc53d3028cfec6961true', 'aria-expanded': 'false', 'aria-disabled': 'false', 'aria-label': 'Details', 'class': 'pf-v5-c-button pf-m-plain', 'type': 'button', 'data-ouia-component-type': 'PF5/Button', 'data-ouia-safe': 'true', 'data-ouia-component-id': 'OUIA-Generated-Button-plain-2'}, 'shadowRoot': None}}}}, {'type': 'pointerDown', 'button': 0}, {'type': 'pointerUp', 'button': 0}]}]}) [id 281]
> warn: Th: Table headers must have an accessible name. If the Th is intended to be visually empty, pass in screenReaderText. If the Th contains only non-text, interactive content such as a checkbox or expand toggle, pass in an aria-label.
> warn: Th: Table headers must have an accessible name. If the Th is intended to be visually empty, pass in screenReaderText. If the Th contains only non-text, interactive content such as a checkbox or expand toggle, pass in an aria-label.
INFO:bidi.command:→ {}
INFO:bidi.command:← script.evaluate({'expression': 'ph_wait_cond(() => ph_is_present(".pf-m-expanded button:contains(\'Integration\'):not([disabled]):not([aria-disabled=true])"), 15000, null)', 'awaitPromise': True, 'target': {'context': '6442450947'}}) [id 282]
INFO:bidi.command:→ {'realm': '00cea0c4-a123-4c6a-85a9-bdaf6e21fa12', 'type': 'success', 'result': None}
INFO:bidi.command:← script.evaluate({'expression': 'ph_wait_cond(() => ph_is_visible(".pf-m-expanded button:contains(\'Integration\'):not([disabled]):not([aria-disabled=true])"), 15000, null)', 'awaitPromise': True, 'target': {'context': '6442450947'}}) [id 283]
INFO:bidi.command:→ {'realm': '00cea0c4-a123-4c6a-85a9-bdaf6e21fa12', 'type': 'success', 'result': None}
INFO:bidi.command:← script.evaluate({'expression': 'ph_find_scroll_into_view(".pf-m-expanded button:contains(\'Integration\'):not([disabled]):not([aria-disabled=true])")', 'awaitPromise': True, 'target': {'context': '6442450947'}}) [id 284]
> warn: Th: Table headers must have an accessible name. If the Th is intended to be visually empty, pass in screenReaderText. If the Th contains only non-text, interactive content such as a checkbox or expand toggle, pass in an aria-label.
INFO:bidi.command:→ {'realm': '00cea0c4-a123-4c6a-85a9-bdaf6e21fa12', 'type': 'success', 'result': {'type': 'node', 'sharedId': 'bb302cef-f186-4076-9063-401e40104efd', 'value': {'nodeType': 1, 'localName': 'button', 'namespaceURI': 'http://www.w3.org/1999/xhtml', 'childNodeCount': 1, 'attributes': {'type': 'button', 'data-ouia-component-type': 'PF5/TabButton', 'data-ouia-safe': 'true', 'class': 'pf-v5-c-tabs__link', 'id': 'pf-tab-1-pf-1724156278142wa0bh1pnou', 'role': 'tab', 'aria-selected': 'false'}, 'shadowRoot': None}}}
INFO:bidi.command:← input.performActions({'context': '6442450947', 'actions': [{'id': 'pointer-285', 'type': 'pointer', 'parameters': {'pointerType': 'mouse'}, 'actions': [{'type': 'pointerMove', 'x': 0, 'y': 0, 'origin': {'type': 'element', 'element': {'type': 'node', 'sharedId': 'bb302cef-f186-4076-9063-401e40104efd', 'value': {'nodeType': 1, 'localName': 'button', 'namespaceURI': 'http://www.w3.org/1999/xhtml', 'childNodeCount': 1, 'attributes': {'type': 'button', 'data-ouia-component-type': 'PF5/TabButton', 'data-ouia-safe': 'true', 'class': 'pf-v5-c-tabs__link', 'id': 'pf-tab-1-pf-1724156278142wa0bh1pnou', 'role': 'tab', 'aria-selected': 'false'}, 'shadowRoot': None}}}}, {'type': 'pointerDown', 'button': 0}, {'type': 'pointerUp', 'button': 0}]}]}) [id 285]
INFO:bidi.command:→ {}
INFO:bidi.command:← script.evaluate({'expression': 'ph_wait_cond(() => ph_is_present("#containers-containers tr:contains(\\"alpine\\") dt:contains(\\"Volumes\\") + dd"), 15000, null)', 'awaitPromise': True, 'target': {'context': '6442450947'}}) [id 286]
> warn: Th: Table headers must have an accessible name. If the Th is intended to be visually empty, pass in screenReaderText. If the Th contains only non-text, interactive content such as a checkbox or expand toggle, pass in an aria-label.
> warn: Th: Table headers must have an accessible name. If the Th is intended to be visually empty, pass in screenReaderText. If the Th contains only non-text, interactive content such as a checkbox or expand toggle, pass in an aria-label.
> warn: Th: Table headers must have an accessible name. If the Th is intended to be visually empty, pass in screenReaderText. If the Th contains only non-text, interactive content such as a checkbox or expand toggle, pass in an aria-label.
----- user containers -----
CONTAINER ID  IMAGE       COMMAND     CREATED     STATUS      PORTS       NAMES
----- system containers -----
+ podman ps -a >&2
CONTAINER ID  IMAGE                                    COMMAND     CREATED         STATUS         PORTS                                                                        NAMES
b6d169519f5a  localhost/podman-pause:5.2.0-1722556800              20 seconds ago  Up 20 seconds  127.0.0.2:37929->9001/tcp, 0.0.0.0:6000->5000/tcp, 127.0.0.1:6001->5001/udp  5ca7d7abed81-infra
e26a27f009ab  localhost/test-alpine:latest             sleep 500   20 seconds ago  Up 20 seconds  127.0.0.2:37929->9001/tcp, 0.0.0.0:6000->5000/tcp, 127.0.0.1:6001->5001/udp  test-pod-1-system
+ for c in $(podman ps -aq); do echo "---- $c ----" >&2; podman logs $c >&2; done
---- b6d169519f5a ----
---- e26a27f009ab ----
Traceback (most recent call last):
  File "/home/jelle/projects/cockpit-podman/./test/check-application", line 2879, in testCreatePodSystem
    self._createPod(True)
  File "/home/jelle/projects/cockpit-podman/./test/check-application", line 2957, in _createPod
    b.wait_in_text('#containers-containers tr:contains("alpine") dt:contains("Volumes") + dd',
  File "/home/jelle/projects/cockpit-podman/test/common/testlib.py", line 838, in wait_in_text
    self.wait_visible(selector)
  File "/home/jelle/projects/cockpit-podman/test/common/testlib.py", line 807, in wait_visible
    self._wait_present(selector)
  File "/home/jelle/projects/cockpit-podman/test/common/testlib.py", line 798, in _wait_present
    self.wait_js_func('ph_is_present', selector)
  File "/home/jelle/projects/cockpit-podman/test/common/testlib.py", line 792, in wait_js_func
    self.wait_js_cond("%s(%s)" % (func, ','.join(map(jsquote, args))))
  File "/home/jelle/projects/cockpit-podman/test/common/testlib.py", line 789, in wait_js_cond
    raise Error(f"timeout\nwait_js_cond({cond}): {last_error.msg}") from None
testlib.Error: timeout
wait_js_cond(ph_is_present("#containers-containers tr:contains(\"alpine\") dt:contains(\"Volumes\") + dd")): Error: condition did not become true

Don't see this in CI, but it is 💯 reproducible locally.

[martinpitt]

These tests are wonky with Chromium as well, but for different reasons. I can reproduce the latter locally (although with Firefox) -- the dialog's primary button just does not want to become active, even though there are no validation errors. This reproduces 100% for me, and it fails earlier than @jelly 's error above, so I suppose I need to fix that first before I can investigate that. (Firefox failed on TF the same way). This is an actual real bug -- even interactively I can't seem to get the "Create" button to enable itself, even after wildly clicking and tabbing around, changing the name, or deleting port and volume mappings.

I manually did a firefox run on our infra and that succeeded -- so these are race conditions, no hard errors.

testRunImageUser fails the same way locally for me as on TF, so I have a lead here. This is actually quite obvious to fix, and it's unclear why this doesn't break with Chromium on CI (it certainly breaks for me locally, and I get the funniest failures). Fix pushed here.

@jelly The "Decrease CPU shares" thing is curious -- CPU shares is disabled by default, so I originally thought it meant to click the checkbox, not the Minus button in the shares value selector. A few lines down it changes the shares to "512", so I don't think it meant to decrease it to 1023. However, the comment says "checkbox is enabled when clicked on the field", so I suppose it's indeed meant to work like that. So your fix is correct. I split it out into a separate commit with some details.

I can locally reproduce the testCreateContainerInPodUser "out of bounds" error. Observations:

• This starts working with a sit() or a time.sleep(0.5). However, I don't see any flicker on rendering, so whatever the race is, it's fast enough to not be humanly visible
• getBoundingClientRect() of the element ({"x":198,"y":323,"width":486,"height":40,"top":323,"right":684,"bottom":363,"left":198}) and its parent ({"x":198,"y":323,"width":486,"height":40,"top":323,"right":684,"bottom":363,"left":198}) both seem reasonable

But I just realized what happens: The dialog defaults to "All", and the busybox image is also displayed there. We click on "Local", but there's nothing that waits for the "new" test-busybox render to happen -- the click could still try to go to the old one before it switches over to the "Local" view. That's why the sleep helps.

[martinpitt]

@jelly So aside from some minor flakes, the main bug is now that forever-disabled Create button in the pod dialog.

That is 100% reliably reproducible for me with

rm -f Test*; TEST_SHOW_BROWSER=1 TEST_BROWSER=firefox test/check-application TestApplication.testCreatePodUser $RUNC -stv

and as I wrote above, neither a race condition, nor a quirk with clicks -- a human can't enable that "Create" button either. I figure this requires some console.log instrumentation why the button is disabled. Do you want to look into that?

[martinpitt]

This testDownloadImage flake (on ubuntu as well) is also fairly persistent. Again, not related to clicking (happens on Chromium), and smells more like "missed podman event". Retrying these, in the meantime I run this in a loop locally to try and reproduce.

[jelly]

Amplified createPod with debug logs:

validate port null IP [PublishPort.jsx:22:12](http://127.0.0.2:9191/cockpit/@localhost/src/PublishPort.jsx)
validate port null hostPort [PublishPort.jsx:22:12](http://127.0.0.2:9191/cockpit/@localhost/src/PublishPort.jsx)
validate port -1 containerPort [PublishPort.jsx:22:12](http://127.0.0.2:9191/cockpit/@localhost/src/PublishPort.jsx)
validationchange publish 
Array [ <1 empty slot> ]
[PodCreateModal.jsx:79:16](http://127.0.0.2:9191/cockpit/@localhost/src/PodCreateModal.jsx)
validate port -1 containerPort [PublishPort.jsx:22:12](http://127.0.0.2:9191/cockpit/@localhost/src/PublishPort.jsx)
validationchange publish 
Array [ {…} ]
[PodCreateModal.jsx:79:16](http://127.0.0.2:9191/cockpit/@localhost/src/PodCreateModal.jsx)
validate port 5000 containerPort [PublishPort.jsx:22:12](http://127.0.0.2:9191/cockpit/@localhost/src/PublishPort.jsx)
validationchange publish 
Array [ {…}, {…} ]
​
0: Object { containerPort: "1 to 65535" }
​
1: Object { containerPort: undefined }

Update: pitti also saw this in a different PR in testCreateContainerInPodSystem here

Update: jelle sees more weird behaviour

So the test first enters -1, waits for validation to appear and then removes the entry. Then add a new entry. Does the state not get cleaned up correctly?

Locally I can reproduce something different but also wrong. Just create a pod, enter -1, click anywhere, validation does not change.

Only when clicking create or adding a new volume

This is an unrelated bug.

[jelly]

Locally one can reproduce this:

State being:

[
    {},
    {
        "containerPort": "Container port must not be empty"
    },
    null,
    {}
]

What I did was, add toy around with the state. Add one valid entry, add an empty, add another valid one, save. The invalid entries aren't cleaned up properly.

Update:

One way to mess things up good:

Easy reproducer:

1. Add new port mapping
2. Add an valid value
3. Add a new port mapping
4. Add an invalid value (-1)
5. Create pod
6. No error message shown..!

[jelly]

So I conclude that we are doing all the state interaction wrong with validation..

The PodCreateModal has a state validationFailed and setValidationFailed setter.

validationFailed is pushed to the DynamicListForm as a property:

            <DynamicListForm id='create-pod-dialog-publish'
                        emptyStateString={_("No ports exposed")}
                        formclass='publish-port-form'
                        label={_("Port mapping")}
                        actionLabel={_("Add port mapping")}
                        validationFailed={validationFailed.publish}
                        onValidationChange={value => dynamicListOnValidationChange('publish', value)}
                        onChange={value => setPublish(value)}
                        default={{ IP: null, containerPort: null, hostPort: null, protocol: 'tcp' }}
                        itemcomponent={ <PublishPort />} />

This component has dynamicListOnValidationChange which is this function that modifies the state:

    const dynamicListOnValidationChange = (key, value) => {
        console.log("validationchange", key, value);
        setValidationFailed(prevState => {
            prevState[key] = value;
            if (prevState[key].every(a => a === undefined))
                delete prevState[key];
            return prevState;
        });
    };

The value passed comes from PublishPort:

                <TextInput id={id + "-container-port"}
                            type='number'
                            step={1}
                            min={1}
                            max={MAX_PORT}
                            validated={validationFailed?.containerPort ? "error" : "default"}
                            value={item.containerPort || ''}
                            onChange={(_event, value) => {
                                console.log("onChange", validationFailed, _event, value);
                                utils.validationClear(validationFailed, "containerPort", onValidationChange);
                                console.log({ ...validationFailed, containerPort: validatePublishPort(value, "containerPort") });
                                utils.validationDebounce(() => onValidationChange({ ...validationFailed, containerPort: validatePublishPort(value, "containerPort") }));
                                onChange(idx, 'containerPort', value);
                            }} />

So this modifies React state and then passes it to onValidationChange which is also a same named function in DynamicListForm:

                                return React.cloneElement(this.props.itemcomponent, {
                                    idx,
                                    item,
                                    id: id + "-" + idx,
                                    key: idx,
                                    onChange: this.onItemChange,
                                    removeitem: this.removeItem,
                                    additem: this.addItem,
                                    options: this.props.options,
                                    validationFailed: validationFailed && validationFailed[idx],
                                    onValidationChange: value => {
                                        // Dynamic list consists of multiple rows. Therefore validationFailed object is presented as an array where each item represents a row
                                        // Each row/item then consists of key-value pairs, which represent a field name and it's validation error
                                        const delta = validationFailed ? [...validationFailed] : [];
                                        // Update validation of only a single row
                                        delta[idx] = value;

                                        // If a row doesn't contain any fields with errors anymore, we delete the item of the array
                                        // Deleting an item of an array replaces an item with an "empty item".
                                        // This guarantees that an array of validation errors maps to the correct rows
                                        if (Object.keys(delta[idx]).length == 0)
                                            delete delta[idx];

                                        onValidationChange?.(delta);
                                    },
                                });

Modifies state again. This seems to be what causes all the inconsistent state issues

[martinpitt]

#1834 landed, rebased to fix the conflict.

[martinpitt]

#1833 landed, pushed rebase to de-conflict.

[jelly]

@martinpitt so this is looking good, do we naughty:

Exiting due to channel error.
Exiting due to channel error.
Exiting due to channel error.
Exiting due to channel error.
Traceback (most recent call last):
  File "/source/test/check-application", line 2896, in testCreatePodSystem
    self._createPod(True)
  File "/source/test/check-application", line 2960, in _createPod
    b.click("#create-pod-create-btn")
  File "/source/test/common/testlib.py", line 568, in click
    self.mouse(selector + ":not([disabled]):not([aria-disabled=true])", "click")
  File "/source/test/common/testlib.py", line 505, in mouse
    self.wait_visible(selector)
  File "/source/test/common/testlib.py", line 807, in wait_visible
    self._wait_present(selector)
  File "/source/test/common/testlib.py", line 798, in _wait_present
    self.wait_js_func('ph_is_present', selector)
  File "/source/test/common/testlib.py", line 792, in wait_js_func
    self.wait_js_cond("%s(%s)" % (func, ','.join(map(jsquote, args))))
  File "/source/test/common/testlib.py", line 789, in wait_js_cond
    raise Error(f"timeout\nwait_js_cond({cond}): {last_error.msg}") from None
testlib.Error: timeout
wait_js_cond(ph_is_present("#create-pod-create-btn:not([disabled]):not([aria-disabled=true])")): Error: condition did not become true

# Result testCreatePodSystem (__main__.TestApplication.testCreatePodSystem) failed

Or adjust the test?

[martinpitt]

@jelly naughty works for now, if this is too hard to work around in the test; can you distill your analysis into an issue? Also, wasn't there another failure in testContainerCreateInPod or so? We talked about it in the chat, too, and you had some idea what went wrong. I don't see a fix for that?

[jelly]

@jelly naughty works for now, if this is too hard to work around in the test; can you distill your analysis into an issue? Also, wasn't there another failure in testContainerCreateInPod or so? We talked about it in the chat, too, and you had some idea what went wrong. I don't see a fix for that?

Hmmm I didn't push a fix, the fix should be quite easy but it no longer reproduces?

[martinpitt]

@jelly how about @skipBrowser("firefox") for the time being?

[martinpitt]

Done, let's see if that helps enough.

[jelly]

Done, let's see if that helps enough.

Sure!

[martinpitt]

@jelly Hm, no, not sufficient: chromium runs into this as well

[mvollmer]

Continued in #1842.