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

removed NodeInfo #1020

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Conversation

mslacken
Copy link
Member

@mslacken mslacken commented Dec 15, 2023

  • removed NodeInfo and using NodeConf instead
  • wwctl comands without NodeInfo
  • api changes for use without NodeInfo
  • fixed tests, mostly will use testenv now

@mslacken mslacken marked this pull request as draft December 15, 2023 15:14
@mslacken
Copy link
Member Author

@anderbubble Hoho,, that was my present, merge the grub changes, so that I will a present

@mslacken mslacken force-pushed the RemoveNodeInfoRebased branch 9 times, most recently from c343a6e to e736cb2 Compare December 20, 2023 21:13
@anderbubble anderbubble added this to the v4.6.0 milestone Feb 17, 2024
@mslacken mslacken force-pushed the RemoveNodeInfoRebased branch 4 times, most recently from 542b32a to f07b774 Compare March 11, 2024 07:30
@mslacken
Copy link
Member Author

close #917

@anderbubble
Copy link
Collaborator

@mslacken I'd like to start helping to get this merged. I tried to do the rebase myself, but I think you'll be more readably able to resolve the conflicts in internal/pkg/node/methods.go than I am.

This is a significant change in the undelying data model!

nodeDb, err := node.New()

will result in a structure which contains the on disk
values. Only

nodeDb.FindAllNodes() or nodeDb.GetNode(id) will give
the nodes with its merged in profiles.

Signed-off-by: Christian Goll <cgoll@suse.com>
mostly remove Get and calls for the id

Signed-off-by: Christian Goll <cgoll@suse.com>
Signed-off-by: Christian Goll <cgoll@suse.com>
Signed-off-by: Christian Goll <cgoll@suse.com>
introduced wwbool and don't export
Nodes and NodeConfs. This requires new
explict Yaml (un)marshaling as the standard
marshaller won't touch these fields

Signed-off-by: Christian Goll <cgoll@suse.com>
this types like WWbool are needed so that they can
have their own command line parses which allows a UNDEF
for e.g. bool or ints.

Signed-off-by: Christian Goll <cgoll@suse.com>
Signed-off-by: Christian Goll <cgoll@suse.com>
Signed-off-by: Christian Goll <cgoll@suse.com>
changes can now not be done directly but must
go to SetNode or SetProfile. Although its also
now possible to access the field direclty with
GetNodePtr

Signed-off-by: Christian Goll <cgoll@suse.com>
Signed-off-by: Christian Goll <cgoll@suse.com>
Comment on lines +21 to +24
NodeProfiles map[string]*ProfileConf
Nodes map[string]*NodeConf
nodeProfiles map[string]*ProfileConf
nodes map[string]*NodeConf
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be consolidated or otherwise updated to not have both exported and non-exported names the same. (I think we can just do everything with the exported names and remove the non-exported, though callers will need to be updated.)

if remoteNode.Kernel.Override != "" {
stage_file = kernel.KernelImage(remoteNode.Kernel.Override)
} else if remoteNode.ContainerName != "" {
stage_file = container.KernelFind(remoteNode.ContainerName)
Copy link
Member

Choose a reason for hiding this comment

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

looks like you need to rebase this branch, the function could not be found during make.

)

type NodeConfDel struct {
TagsDel []string `lopt:"tagdel" comment:"add tags"`
Copy link
Member

Choose a reason for hiding this comment

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

the comment should be delete tags

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge NodeConf and NodeInfo
3 participants