Skip to content

Add server/config package.#320

Open
Chounoki wants to merge 8 commits into
openconfig:mainfrom
Chounoki:pr3
Open

Add server/config package.#320
Chounoki wants to merge 8 commits into
openconfig:mainfrom
Chounoki:pr3

Conversation

@Chounoki
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new configuration schema for the Bootz server, including Protobuf definitions, Bazel build rules, and generated Go code. The review feedback focuses on improving the Protobuf design and ensuring consistent Go package naming. Specifically, it is recommended to use the bytes type instead of Base64-encoded strings for binary data like certificates and keys to be more idiomatic and efficient. Additionally, the reviewer suggested adding option go_package to the proto file and updating the Bazel importpath to ensure the generated Go package name correctly reflects the intended structure.

Comment thread server/proto/BUILD.bazel Outdated
Comment thread server/proto/config/config.pb.go Outdated
Comment thread server/proto/config.proto
Comment thread server/proto/config.proto Outdated
Comment thread server/proto/config.proto
Comment thread server/proto/config.proto
@Chounoki Chounoki force-pushed the pr3 branch 2 times, most recently from f673437 to 7ef981f Compare May 18, 2026 10:07
@Chounoki Chounoki requested a review from gmacf May 18, 2026 10:16
@gmacf
Copy link
Copy Markdown
Contributor

gmacf commented May 19, 2026

Is this for Monax? If so, I would like to see how this fits in with the existing protos or proposed workflow. Do you have any example configs for this?

@Chounoki
Copy link
Copy Markdown
Contributor Author

Is this for Monax? If so, I would like to see how this fits in with the existing protos or proposed workflow. Do you have any example configs for this?

Yes, this is for Monax, so the vendors can use all real data like ownership_voucher.
I haven't created the PR yet, but when creating the Bootz server, this Config proto should be read from a file and provided to the Bootz server.

Comment thread server/proto/config.proto
// A binding configuration for Bootz server.
message Config {
// Bootz server address (IP:port).
string server_address = 1;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How will we know the server address of the Bootz server ahead of time? Some runtime environments (e.g. Google's) don't let you reserve a specific IP when bringing up SUT components. We will need another way to learn about the Bootz server address.

Copy link
Copy Markdown
Contributor Author

@Chounoki Chounoki May 19, 2026

Choose a reason for hiding this comment

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

Here is my understanding:

With Monax, each SUT will run in a container, which by default is assigned to a private network by Docker runtime. This private network is bridged (NAT'ed) by the host machine. (I.e. the host machine acts as the gateway of the SUT's private network).

https://docs.docker.com/engine/network/

This private network address doesn't really matter, because any external switch will only see the real IP address of the host machine on which the containers run. In other words, the host machine does port mapping from container private network to host public network.

The DHCP entries must use the real host IP address as Bootz server address. Similarly the image URI created by Bootz must also use the real host IP address, because to any external switches, only the real IP address of the host machine is visible (All IP addresses inside the private network of the containners are not visible externally).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Monax isn't necessarily going to run on Docker containers. Some vendors may run these SUT components on bare-metal machines, and as I said above, it definitely wouldn't work in Borg. This is why we should find a solution that can be applied to any runtime/deployment environment.

Copy link
Copy Markdown
Contributor Author

@Chounoki Chounoki May 21, 2026

Choose a reason for hiding this comment

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

If the vendors are going to directly run the Monax test we provide to them, then each SUT will be inside a container just like the math example provided by Monax, you can see each SUT has its own Dockerfile and kubernetes.txtpb file.

If the vendors don't plan to run the Monax, but to use each service on bare machine without Monax, then they already know the IP of the machine which can be manually written into the textproto of this file by the vendors, and then Bootz server will be initialized from this textproto.

In either case, personally I think it is better and safer to ask the vendors to manually fill in the IP address of the machine on which they plan to run the test (regardless of Bootz server inside a container or not), since the machine can have multiple network interfaces and multiple IP addresses.

While Bootz calls net.Listen() function without specifying an IP, it will create a listener on each of those network interfaces and their IP addresses (https://pkg.go.dev/net#Listen), which means if we try to retrieve a single server IP address dynamically from Bootz runtime, we would get multiple IP addresses instead, and it would be a mess because we can only put one single Bootz IP into DHCP.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think my main point still stands: How will we run this Monax test on Borg if you have to know the IP address ahead of time? We need to think about all future cases, not just the immediate ones.

Monax at its core is a SUT manager. It's purpose is to automatically bring up, manage and provide access to SUT components without having to know the implementation/addressing details about them. It is responsible for figuring out the IP addresses of the SUT components it brings up, and then initiating the gRPC/HTTP/whatever connections to them. By introducing external fields that override this, you are potentially breaking that binding.

Regarding net.Listen() listening on all available IP addresses, I don't see that as a huge problem. Sure, the server will need to pick one address to provide in the DHCP lease but the beauty of Monax is that each vendor can customize that logic as much as they want. This is why I've put in a GetBootzURL() RPC to the Bootz controller: Each test implementation has full control over how the Bootz server determines what its correct address is.

Comment thread server/proto/config.proto Outdated
Comment thread server/proto/config.proto
// Whether Streaming Bootz is supported or not.
bool streaming_supported = 5;
// Software image to be loaded on the chassis.
bootz.SoftwareImage intended_image = 6;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The Bootz SoftwareImage contains a download URL. How will we know that URL ahead of time? This URL should be generated by the ImageService SUT dynamically.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the image URI must also use the real IP address of the host machine, which is known by the user doing the test beforehand.

In the worst case, Bootz can still override this IP if required (i.e. we can provide a function for Monax to call).

@gmacf
Copy link
Copy Markdown
Contributor

gmacf commented May 19, 2026

Is this for Monax? If so, I would like to see how this fits in with the existing protos or proposed workflow. Do you have any example configs for this?

Yes, this is for Monax, so the vendors can use all real data like ownership_voucher. I haven't created the PR yet, but when creating the Bootz server, this Config proto should be read from a file and provided to the Bootz server.

In that case, could you either edit the existing Monax protos under server/tests/proto or move the config there? The reason I ask is because this file is very similar to server/tests/proto/test.proto

@Chounoki
Copy link
Copy Markdown
Contributor Author

Chounoki commented May 19, 2026

Is this for Monax? If so, I would like to see how this fits in with the existing protos or proposed workflow. Do you have any example configs for this?

Yes, this is for Monax, so the vendors can use all real data like ownership_voucher. I haven't created the PR yet, but when creating the Bootz server, this Config proto should be read from a file and provided to the Bootz server.

In that case, could you either edit the existing Monax protos under server/tests/proto or move the config there? The reason I ask is because this file is very similar to server/tests/proto/test.proto

Besides Monax, this proto definition will be referenced by artifact_manager and chassis_manager as argument in their respective New() function, as well as by server_emulator and client_emulator, so I am not sure whether putting it into the test directory is appropriate.

Even without doing any test (Monax or not), this proto definition is still required.

@gmacf
Copy link
Copy Markdown
Contributor

gmacf commented May 20, 2026

Is this for Monax? If so, I would like to see how this fits in with the existing protos or proposed workflow. Do you have any example configs for this?

Yes, this is for Monax, so the vendors can use all real data like ownership_voucher. I haven't created the PR yet, but when creating the Bootz server, this Config proto should be read from a file and provided to the Bootz server.

In that case, could you either edit the existing Monax protos under server/tests/proto or move the config there? The reason I ask is because this file is very similar to server/tests/proto/test.proto

Besides Monax, this proto definition will be referenced by artifact_manager and chassis_manager as argument in their respective New() function, as well as by server_emulator and client_emulator, so I am not sure whether putting it into the test directory is appropriate.

Even without doing any test (Monax or not), this proto definition is still required.

I'm not sure how this config proto will be used in the production (non-test instance). Doesn't this config have to contain an exhaustive list of the inventory and other artifacts? In the real world, we fetch these from external systems.

@Chounoki
Copy link
Copy Markdown
Contributor Author

Chounoki commented May 21, 2026

Is this for Monax? If so, I would like to see how this fits in with the existing protos or proposed workflow. Do you have any example configs for this?

Yes, this is for Monax, so the vendors can use all real data like ownership_voucher. I haven't created the PR yet, but when creating the Bootz server, this Config proto should be read from a file and provided to the Bootz server.

In that case, could you either edit the existing Monax protos under server/tests/proto or move the config there? The reason I ask is because this file is very similar to server/tests/proto/test.proto

Besides Monax, this proto definition will be referenced by artifact_manager and chassis_manager as argument in their respective New() function, as well as by server_emulator and client_emulator, so I am not sure whether putting it into the test directory is appropriate.
Even without doing any test (Monax or not), this proto definition is still required.

I'm not sure how this config proto will be used in the production (non-test instance). Doesn't this config have to contain an exhaustive list of the inventory and other artifacts? In the real world, we fetch these from external systems.

This config profo is only for the open source Bootz. This config proto itself as well as its consumers: artifact_manager, chassis_manager, bootz_emulator and client_emulator won't be sync'ed back to our internal codebase.

Our internal Bootz will use a different version of artifact_manager and chassis_manager, which implements the same interfaces as the open source ones.

@Chounoki
Copy link
Copy Markdown
Contributor Author

Chounoki commented May 21, 2026

Is this for Monax? If so, I would like to see how this fits in with the existing protos or proposed workflow. Do you have any example configs for this?

Yes, this is for Monax, so the vendors can use all real data like ownership_voucher. I haven't created the PR yet, but when creating the Bootz server, this Config proto should be read from a file and provided to the Bootz server.

In that case, could you either edit the existing Monax protos under server/tests/proto or move the config there? The reason I ask is because this file is very similar to server/tests/proto/test.proto

Besides Monax, this proto definition will be referenced by artifact_manager and chassis_manager as argument in their respective New() function, as well as by server_emulator and client_emulator, so I am not sure whether putting it into the test directory is appropriate.
Even without doing any test (Monax or not), this proto definition is still required.

I'm not sure how this config proto will be used in the production (non-test instance). Doesn't this config have to contain an exhaustive list of the inventory and other artifacts? In the real world, we fetch these from external systems.

This config profo is only for the open source Bootz. This config proto itself as well as its consumers: artifact_manager, chassis_manager, bootz_emulator and client_emulator won't be sync'ed back to our internal codebase.

Our internal Bootz will use a different version of artifact_manager and chassis_manager that implement the same interface.

I have created another PR of artifact_manager to demonstrate how this config proto is consumed, but due to the dependency, I have to create the new PR containing both artifact_manager package and this config proto package together in the same PR, otherwise the new PR will fail the building test due to dependency.

@Chounoki
Copy link
Copy Markdown
Contributor Author

Chounoki commented May 21, 2026

I think we can put Monax aside for now, because we are still too far away from implementing any Monax test.

The major use of this config proto definition is to replace the bootz server startup argument defined here:
https://github.com/openconfig/bootz/blob/main/server/emulator/main.go#L38-L42

I will remove all those command line arguments and only use a textproto file generated from this config proto definition as the command line argument for starting a bootz server emulator.

Similarly the client emulator will do the same also using the same textproto file generated from this config proto definition as the command line argument for starting a bootz client emulator.

How we should start the Monax Bootz test can be discussed later and doesn't necessarily have to use this config proto.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants