SUSE / salt-shaptools

Salt execution module and state to manage SAP Applications (HANA only at the moment) and SUSE Linux Enterprise High Availability components
Apache License 2.0
14 stars 11 forks source link

Add saptune functionality for solution apply #45

Closed MalloZup closed 4 years ago

MalloZup commented 4 years ago

Things to do:

MalloZup commented 4 years ago

@arbulu89 from you pov a part of the unit-tests for the states, is something else I'm missing here? tia

angelabriel commented 4 years ago

And some optional: get_solution_list

Only a heads up - 'saptune solution list' will give you the list of all available solutions and their related SAP Notes. But if one solution is already applied, this line is highlighted in 'green' color. So you will have some control characters (like ^[[32m) in the output of the command.

Machine readable output of all saptune actions is on our wish list, but currently under discussion and not yet implemented.

angelabriel commented 4 years ago

I have a new concern. What are we going to do if one solution is already applied? I would assume that the current solution is reverted and the new one applied (as only 1 solution can exist). This would make the whole logic more complex (except saptune binary is able to manage that, what I don't know). I would vote for having this logic (to assure idempotency). What do you think @MalloZup and @angelabriel ?

As we are in a deployment situation, I agree, that this will be the way to handle this situation. But in a productive SAP environment (so outside of a 'deployment') I would never recommend to automatically revert an already applied solution and apply another one, as you never know which performance impact this will have. Only an administrator should do that (and hopefully he will know what he is doing)

MalloZup commented 4 years ago

@angelabriel @arbulu89 Regarding the situation of already applied solution,

I think the best thing to implement is:

arbulu89 commented 4 years ago

@angelabriel @arbulu89 Regarding the situation of already applied solution,

I think the best thing to implement is:

* if there is already a solution applied the call of the module function should error out sayng that there is already a solution applied. In that case the admin should revert the solution before and apply another one

I think currently is the best solution, in the future I would add a new param like force to revert the current solution and apply the new, this is a really common scenario in salt

MalloZup commented 4 years ago

regarding the UT, i already tried to look at the existing codebase but personally I think that there is no clear emerging pattern for a nob, in the codebase I felt we have diverging pattern even in the unit-test (here). I will try re-look and perhaps improve doc if I can. thx for pointers!

MalloZup commented 4 years ago

@arbulu89 I think is good to merge right now. I have update the test etc. let me know! tia! :rocket:

MalloZup commented 4 years ago

thx! i will merge after done with manual test

MalloZup commented 4 years ago

tested worked fine