Skip to content

Ensure empty build cache is represented as empty JSON array#42607

Merged
thaJeztah merged 2 commits intomoby:masterfrom
rvolosatovs:fix_build_cache_formatting
Jul 9, 2021
Merged

Ensure empty build cache is represented as empty JSON array#42607
thaJeztah merged 2 commits intomoby:masterfrom
rvolosatovs:fix_build_cache_formatting

Conversation

@rvolosatovs
Copy link

@rvolosatovs rvolosatovs commented Jul 8, 2021 •
edited
Loading

Closes #42605 Refs #42605 (comment)

- What I did
Ensure empty BuildCache field is represented as empty JSON array([]) instead of null to be consistent with Images, Containers etc.

- How I did it
Initialize the struct field with an empty slice if buildCache is nil.
Note, the empty slice could be potentially defined as global variable and reused to avoid unnecessary allocations, but that is an optimization not worth doing here in my opinion

- How to verify it
curl -s --unix-socket /var/run/docker.sock http://localhost/system/df | jq on empty daemon should output:

{
  "LayersSize": 0,
  "Images": [],
  "Containers": [],
  "Volumes": [],
  "BuildCache": [],
  "BuilderSize": 0
}

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

API: ensure empty build cache is represented as empty JSON array
f222593 
Ensure empty `BuildCache` field is represented as empty JSON array(`[]`)
instead of `null` to be consistent with `Images`, `Containers` etc.

Signed-off-by: Roman Volosatovs <roman.volosatovs@docker.com>
@rvolosatovs rvolosatovs mentioned this pull request Jul 8, 2021
Closed
thaJeztah
thaJeztah approved these changes Jul 8, 2021
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@thaJeztah thaJeztah added this to the 21.xx milestone Jul 8, 2021
cpuguy83
cpuguy83 approved these changes Jul 8, 2021
View reviewed changes
Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

thaJeztah
thaJeztah requested changes Jul 9, 2021
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you fix the trailing comma's in the first commit?

@thaJeztah
swagger: add BuildCache example to /system/df
83e3dd6 
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Roman Volosatovs <roman.volosatovs@docker.com>
@rvolosatovs rvolosatovs force-pushed the fix_build_cache_formatting branch from 8946769 to 83e3dd6 Compare July 9, 2021 07:58
@rvolosatovs rvolosatovs requested a review from thaJeztah July 9, 2021 07:58
@thaJeztah thaJeztah mentioned this pull request Jul 9, 2021
Closed
@thaJeztah
Copy link
Member

thaJeztah commented Jul 9, 2021

Flaky test; tracked through #42482

=== RUN   TestFindNetworkUtil
time="2021-07-09T08:17:15Z" level=warning msg="bridge store not initialized. kv object docker/network/v1.0/bridge/710e955a31b7786099e4e4e518e078035313460d5939608bcef973d4923a6ed0/ is not added to the store"
time="2021-07-09T08:17:15Z" level=warning msg="Failed to update store after ipam release for network network (710e955a31b7786099e4e4e518e078035313460d5939608bcef973d4923a6ed0): failed to update store for object type *libnetwork.network: Key not found in store"
    api_linux_test.go:1099: Failed to delete the network: error deleting network endpoint count from store: Key not found in store
--- FAIL: TestFindNetworkUtil (0.15s)

Flaky test on Windows 2022 (created #42612)

=== RUN   TestRequestReleaseAddressDuplicate
    allocator_test.go:1533: IP 198.168.1.22/23 was previously allocated
--- FAIL: TestRequestReleaseAddressDuplicate (0.04s)

thaJeztah
thaJeztah approved these changes Jul 9, 2021
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member

thaJeztah commented Jul 9, 2021

One more; was flaky before (#38293, also mentioned in #41561), but hadn't seen that one in a while

=== RUN   TestRenameAnonymousContainer
    rename_test.go:167: assertion failed: 0 (int) != 1 (inspect.State.ExitCode int): container 3d1d8bd2f8e1ce460b713ff313de83ac0f5c83fa8eb8dd5c0864bbf9f17cc60e exited with the wrong exitcode:
--- FAIL: TestRenameAnonymousContainer (10.08s)

kicked CI again

@thaJeztah
Copy link
Member

thaJeztah commented Jul 9, 2021

Ignoring the TestNetworkDBIslands; green otherwise; let's merge

@thaJeztah thaJeztah merged commit c81abef into moby:master Jul 9, 2021
@gesellix gesellix mentioned this pull request Jul 13, 2021
Open
mfeit-internet2 pushed a commit to mfeit-internet2/moby that referenced this pull request Jul 13, 2021
@thaJeztah
Merge pull request moby#42607 from rvolosatovs/fix_build_cache_format…
bebb2db 
…ting

Ensure empty build cache is represented as empty JSON array
Signed-off-by: Mark Feit <mfeit@internet2.edu>
@rvolosatovs rvolosatovs mentioned this pull request Jul 22, 2021
Merged
@rvolosatovs rvolosatovs deleted the fix_build_cache_formatting branch July 22, 2021 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cpuguy83 cpuguy83 cpuguy83 approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees

No one assigned

Labels

area/api API status/4-merge

Projects

None yet

Milestone

23.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@rvolosatovs @thaJeztah @cpuguy83