Conversation
| if meta := node.GetCloudMetadata(); meta != nil && meta.AWS != nil { | ||
| names = append(names, types.ServerInfoNameFromAWS(meta.AWS.AccountID, meta.AWS.InstanceID)) | ||
| } | ||
| // Manually added ServerInfos should override any other ServerInfos. |
There was a problem hiding this comment.
I get the comment, but I don't get it in the context of this method. What manually added infos do we see here?
There was a problem hiding this comment.
I believe that "manually added ServerInfos" are the node-name-based ones. The AWS ones are created by the discovery process in lib/srv/server/ec2_watcher.go. Not sure where it is best to record this information in a comment - perhaps this could say:
// Node name based ServerInfos are manually added and override the AWS ServerInfos automatically
// added by discovery. Labels from ServerInfos later in the list will override the earlier ones.
or something like that perhaps.
There was a problem hiding this comment.
I've removed the "manually added" language since the matching method is really the only thing that determines the order.
camscale
left a comment
camscale
left a comment
There was a problem hiding this comment.
Mostly just style comments that would make things a little clearer/cleaner IMHO
| if meta := node.GetCloudMetadata(); meta != nil && meta.AWS != nil { | ||
| names = append(names, types.ServerInfoNameFromAWS(meta.AWS.AccountID, meta.AWS.InstanceID)) | ||
| } | ||
| // Manually added ServerInfos should override any other ServerInfos. |
There was a problem hiding this comment.
I believe that "manually added ServerInfos" are the node-name-based ones. The AWS ones are created by the discovery process in lib/srv/server/ec2_watcher.go. Not sure where it is best to record this information in a comment - perhaps this could say:
// Node name based ServerInfos are manually added and override the AWS ServerInfos automatically
// added by discovery. Labels from ServerInfos later in the list will override the earlier ones.
or something like that perhaps.
* Add second server info type to reconciler * Add server info tctl CRUD * Fix parsing * Add test * Remove unused constant * Fix EC2 server info names * Extract server info names to functions * Fix test * Address comments * Fix test * Fix error message
* Add second server info type to reconciler * Add server info tctl CRUD * Fix parsing * Add test * Remove unused constant * Fix EC2 server info names * Extract server info names to functions * Fix test * Address comments * Fix test * Fix error message
3 participants
This change adds the ability for users to create ServerInfo resources with
tctl.In addition to matching a node by its AWS info, a ServerInfo can now match a node by its ID. For a node with name
<name>, the ServerInfo that matches it should be namedsi-<name>. If both types of ServerInfo exist, the labels provided by both will be merged, with labels from the ID ServerInfo taking precedent if they conflict.Resolves #31261.
Changelog: Exposed ServerInfo resource in
tctl