API for node_register

[xuhang57]

This patch adds the API function for node_register. The cli code change accordingly.

Currently, obm_types = ["ipmi", "mock"] is a hardcoded variable. A discussion is needed on whether or not to keep the hardcoded variable.

Any idea/suggestion/opinion/ is more than welcome.

Consider this is a WIP.

This relates to the issue: https://github.com/CCI-MOC/hil/issues/924

[xuhang57]

one of the ideas to improve the code is to store obm_api as a global variable somewhere so we can use self.obm_api to get the uri (url?). I am still new to the code base. I guess we can probably put it in the base.py under hil/client/? Any idea would be much appreciated.

[xuhang57]

@SahilTikale @naved001 @Izhmash @zenhack any suggestion/discussion would be much appreciated. I don't know whether I am on the right track and is there any improvements I could make? Thanks!

[zenhack]

Worth noting, once the obmd stuff lands, node_register won't take a type at all, because the differences between OBMs will be handled by obmd. So I wouldn't fuss about it right now.

This of course pushes the question about how to deal with this to whatever software we end up using to talk to obmd, but that doesn't go in HIL's node_register.

[xuhang57]

So the obmd would only affect the node_register right? As for other items in the list such as switch_register, we should be able to proceed?

As for the node_register, do we need a temporary solution for the client api? If not, what would be the timeline for having the obmd in place? Otherwise, hardcode the type and leave a note along with the change would be a good solution.

Thanks for the inputs guys.

[naved001]

@zenhack

Worth noting, once the obmd stuff lands, node_register won't take a type at all, because the differences between OBMs will be handled by obmd. So I wouldn't fuss about it right now.

Good point. Do you think it's still worth moving things into the client library for now since the work has already been done? Like how @xuhang57 suggested that we hardcode the type and leave a note that that call will be changed when obmd is in.

This of course pushes the question about how to deal with this to whatever software we end up using to talk to obmd, but that doesn't go in HIL's node_register.

I guess, we could write a basic CLI to talk to obmd and handle these cases for obmd's register call in the cli itself. Since that will be admin only operations and not really user facing.

@xuhang57

So the obmd would only affect the node_register right? As for other items in the list such as switch_register, we should be able to proceed?

Yes and yes. And I think my point about handling subtypes and payload still stands.

[zenhack]

Right, the obmd stuff only affects node_register. I'm fine just hard-coding stuff in the meantime.

[xuhang57]

@zenhack @naved001 Currently, I am querying the obm type from user's settings rather than hardcoding them. How's that? If fine, code review would be much appreciated!

[naved001]

@zenhack @xuhang57 I am not still not sure why we need the list_active_extensions API. Wish I had spoken earlier about it when we merged it, but I am having doubts now.

[xuhang57]

ok, before I make some changes on the raise exception, I am inclined to hard code the obm types after discussing with Naved. @SahilTikale has asked about this and the reason why we are coding is since the api server will reject the invalid type s anyways, there is no need to check the active extension in the client library again.

Will update the patch accordingly.

[naved001]

Just to add what @xuhang57 said:

My idea is that, even if we get the active OBM types (or in case of switch_register, the active switch types), we still need to know, for each active backend, how to make the request body.

[zenhack]

Yeah, and furthermore it seems unlikely that someone using *_register would not know a priori how HIL was configured -- these calls are admin-only after all. and if they do need to be able to get the info dynamically, they can just respond to the failures. I kindof agree that list_active_extensions seems unnecessary. I think I vaguely remember feeling that way at the time, but didn't have the energy to argue with folks. Do we know of any code actually using it? I'm fine removing it entirely (if we want to be cautious we could mark it as deprecated for a while).

[xuhang57]

I don't see anywhere in the code base has used that function except for the tests. And yeah, should mark it as deprecated for 2 releases?

[naved001]

My vote is to remove that call entirely. @SahilTikale I remember you opening the issue for having that API, do you have any strong argument for keeping it?

[xuhang57]

Will ping Sahil during the weekly meeting.

[xuhang57]

@zenhack @naved001 Done.

[xuhang57]

@naved001 @zenhack done.

[xuhang57]

@zenhack @naved001 done.

[xuhang57]

@naved001 please merge it if you think it is good to go ;)