Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Containerd hosts config are sorted and not in the specified order #549

Closed
vflaux opened this issue Jul 29, 2024 · 3 comments · Fixed by #601
Closed

Containerd hosts config are sorted and not in the specified order #549

vflaux opened this issue Jul 29, 2024 · 3 comments · Fixed by #601
Labels
bug Something isn't working

Comments

@vflaux
Copy link

vflaux commented Jul 29, 2024

Spegel version

v0.0.23

Kubernetes distribution

na

Kubernetes version

na

CNI

na

Describe the bug

The order of hosts in the hosts.toml files in containerd config generated by the spegel configuration command do not preserve the order of previous config nor the order in arguments.
This is due to the fact that maps marshalled with the go-toml library are sorted by key.
The issue is that the order of the "hosts" table is important in containerd as hosts are tried in order of apparition in the file.

All current tests pass because it seems they take this fact into account.
You can try to switch the order of mirrors in input of a test without changing the expected output. The test will still pass while it shouldn't.

@vflaux vflaux added the bug Something isn't working label Jul 29, 2024
@phillebaba
Copy link
Member

You are right it seems like the value of the key decides the ordering of the hosts, which is why the tests would pass.

@phillebaba
Copy link
Member

@vflaux I have thought about it and I think easiest to understand solution is to us Go templates. No need to use embeddings I prefer using a static string to keep things simple. Your initial test to show the problem should be enough to verify things are working as they should. Do you want to do this or can I?

@vflaux
Copy link
Author

vflaux commented Aug 29, 2024

I updated #557 and get ride of the embedded file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants