Systems: Re-enable the modify_interface call #2921

SchoolGuy · 2022-01-28T12:08:59Z

This reintroduces modify_interface to the System object. It is a
compatibility function. The way to do this now is:

system.interfaces[<name>].property = value

The old way was:

system.modify_interface({"property-<name>": value})

However since the new way is not yet supported by all consumers of
the API this compatibility layer.

This fixes #2846, #2896

SchoolGuy · 2022-01-28T12:09:20Z

@holmesb Please confirm that this is working if you have the time.

holmesb · 2022-01-28T13:23:38Z

Saving returns true now, but system still lacks the interface I'm adding:

In [3]: import xmlrpc.client                                                                                                   
   ...: remote=xmlrpc.client.Server("http://localhost:8081/cobbler_api")                                                       
   ...: token=remote.login("cobbler", "cobbler")                                                                               
   ...: system_id = remote.new_system(token)                                                                                   
   ...: remote.modify_system(system_id,"name", "test", token)                                                                  
   ...: remote.modify_system(system_id,"profile", "Ubuntu-20.04-x86_64", token)                                                
   ...: remote.modify_system(system_id,'modify_interface', {"macaddress-eth0" : "aa:bb:cc:dd:ee:ff"}, token)                   
   ...: remote.save_system(system_id, token)                                                                                   
Out[3]: True

But only interface is default, not eth0:

3661167e6ec7:/code # cobbler system report
Name                           : test
<snip>
Interface =====                : default
Bonding Opts                   :
Bridge Opts                    :
CNAMES                         : []
InfiniBand Connected Mode      : False
DHCP Tag                       :
DNS Name                       :
Per-Interface Gateway          :
Master Interface               :
Interface Type                 : NA
IP Address                     :
IPv6 Address                   :
IPv6 Default Gateway           :
IPv6 MTU                       :
IPv6 Prefix                    :
IPv6 Secondaries               : []
IPv6 Static Routes             : []
MAC Address                    :
Management Interface           : False
MTU                            :
Subnet Mask                    :
Static                         : False
Static Routes                  : []
Virt Bridge

Same in v3.2.2 works fine:

root@ed59828114cc:/# cobbler system report
Name                           : test
<snip>
Interface =====                : eth0
Bonding Opts                   :
Bridge Opts                    :
CNAMES                         : []
InfiniBand Connected Mode      : False
DHCP Tag                       :
DNS Name                       :
Per-Interface Gateway          :
Master Interface               :
Interface Type                 : na
IP Address                     :
IPv6 Address                   :
IPv6 Default Gateway           :
IPv6 MTU                       :
IPv6 Prefix                    :
IPv6 Secondaries               : []
IPv6 Static Routes             : []
MAC Address                    : aa:bb:cc:dd:ee:ff
Management Interface           : False
MTU                            :
Subnet Mask                    :
Static                         : False
Static Routes                  : []
Virt Bridge                    :

SchoolGuy · 2022-01-28T13:26:54Z

@holmesb Do you have on 3.2.2 also the interface "default"? Depending on that I could correct the fix.

holmesb · 2022-01-28T13:28:00Z

No, only eth0

SchoolGuy · 2022-01-28T13:29:56Z

@holmesb Oh okay that is a bug too in my eyes. However I found my logic error. I hope that the new version of the commit fixes the behavior now. I will create a test.

codecov · 2022-01-28T13:32:37Z

Codecov Report

Merging #2921 (50a6d3f) into master (c8e4af8) will decrease coverage by 0.20%.
The diff coverage is 40.80%.

@@            Coverage Diff             @@
##           master    #2921      +/-   ##
==========================================
- Coverage   46.96%   46.76%   -0.21%     
==========================================
  Files         101      100       -1     
  Lines       14459    14447      -12     
==========================================
- Hits         6791     6756      -35     
- Misses       7668     7691      +23

Impacted Files	Coverage Δ
cobbler/modules/authentication/ldap.py	`89.15% <0.00%> (-0.13%)`	⬇️
cobbler/modules/installation/post_power.py	`36.36% <0.00%> (ø)`
cobbler/modules/installation/post_puppet.py	`21.21% <0.00%> (ø)`
cobbler/modules/installation/post_report.py	`16.32% <0.00%> (-0.70%)`	⬇️
cobbler/remote.py	`0.00% <0.00%> (ø)`
cobbler/validate.py	`67.62% <ø> (-2.75%)`	⬇️
cobbler/modules/installation/post_log.py	`29.41% <10.00%> (-10.59%)`	⬇️
cobbler/modules/installation/pre_log.py	`29.41% <11.11%> (-6.96%)`	⬇️
cobbler/modules/serializers/mongodb.py	`30.76% <11.11%> (-0.27%)`	⬇️
cobbler/items/system.py	`75.05% <28.57%> (-0.84%)`	⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8487eab...50a6d3f. Read the comment docs.

SchoolGuy · 2022-01-28T13:42:44Z

Last force push. I promise! This time we have it confirmed via the test that our usecase is working.

holmesb · 2022-01-28T13:49:11Z

Yes that's fixed. Both default and eth0 interfaces present. Amazing @SchoolGuy! :-) :-) :-)

SchoolGuy · 2022-01-28T13:50:29Z

Now it is down to my fellow colleagues to rate if they find the code acceptable! :)

holmesb · 2022-01-29T16:06:23Z

Looks like delete_interface isn't working still.

remote.modify_system(system_id,'delete_interface', {"interface" : "eth0"}, token)
True
remote.save_system(system_id, token)
True

But system still has eth0 attached

SchoolGuy · 2022-01-29T19:30:09Z

@holmesb Thanks. Will have a look tomorrow or Monday.

SchoolGuy · 2022-01-30T10:23:06Z

@holmesb Had a look at the logic. It seems that rename_interface also didn't work as expected. I will adjust the commit and add tests. Tests I will do tomorrow though.

SchoolGuy · 2022-01-31T13:58:20Z

I will not be able to fix this today due to some unforeseen events. I will definitely have time tomorrow though.

This reintroduces modify_interface to the System object. It is a compatibility function. The way to do this now is: system.interfaces[<name>].property = value The old way was: system.modify_interface({"property-<name>": value}) However since the new way is not yet supported by all consumers of the API this compatibility layer. This commit as well reintroduces "rename_interface". The usage didn't change for XML-RPC API consumers, however it does for Python API users. An example for XML-RPC: remote.modify_system( system, "rename_interface", {"interface": "eth0", "rename_interface": "eth_new"}, token, ) Lastly this commit also reintroduces "delete_interface". The usage didn't change for XML-RPC API users but it did for Python API users. An example for the XML-RPC can be viewed below: remote.modify_system( system, "delete_interface", {"interface": "eth0"}, token )

SchoolGuy · 2022-02-01T07:23:55Z

@holmesb Should all be fine now. If time permits please give feedback. The commit message should be extensive enough to explain what my commit does. If not feel free to ask.

holmesb · 2022-02-01T08:40:10Z

delete_interface working fine now. Am I right to say rename_interface should look like:

remote.modify_system(system_id,'rename_interface', {"interface" : "eth0"}, {"rename_interface" : "eth1"}, token)
?
If so, it's erroring:
Fault: <Fault 1: "<class 'TypeError'>:modify_system() takes 5 positional arguments but 6 were given">

SchoolGuy · 2022-02-01T08:47:34Z

@holmesb Nope. Look at the commit message. You need something like this:

remote.modify_system(
    system_id,
    'rename_interface',
    {"interface" : "eth0", "rename_interface" : "eth1"},
    token
)

holmesb · 2022-02-01T08:56:07Z

Ah cool, yes working too. Nice work.

SchoolGuy · 2022-02-03T07:14:33Z

Since noone is objecting and this is reportedly working as expected now, I will merge this one with my Admin rights.

SchoolGuy added this to the v3.3.1 milestone Jan 28, 2022

SchoolGuy requested a review from a team January 28, 2022 12:09

SchoolGuy self-assigned this Jan 28, 2022

SchoolGuy force-pushed the bug/fix-interface-modification branch from b2606de to 0d9df4d Compare January 28, 2022 13:29

SchoolGuy force-pushed the bug/fix-interface-modification branch from 0d9df4d to c6faf93 Compare January 28, 2022 13:42

SchoolGuy force-pushed the bug/fix-interface-modification branch from c6faf93 to 5bdda71 Compare January 30, 2022 10:30

SchoolGuy marked this pull request as draft January 30, 2022 10:30

SchoolGuy force-pushed the bug/fix-interface-modification branch from 5bdda71 to b10a4be Compare February 1, 2022 07:23

SchoolGuy marked this pull request as ready for review February 1, 2022 07:23

SchoolGuy force-pushed the bug/fix-interface-modification branch 3 times, most recently from bb02bc3 to 849c75f Compare February 1, 2022 07:45

Docker: Switch from CentOS 8 to Rocky Linux

50a6d3f

SchoolGuy force-pushed the bug/fix-interface-modification branch from 849c75f to 50a6d3f Compare February 1, 2022 07:51

SchoolGuy merged commit a365863 into master Feb 3, 2022

SchoolGuy deleted the bug/fix-interface-modification branch February 3, 2022 07:14

SchoolGuy mentioned this pull request May 1, 2023

XML-RPC API documentation on the Wiki is not up to date for version 3.3.0. #2896

Closed

SchoolGuy linked an issue May 1, 2023 that may be closed by this pull request

XML-RPC API documentation on the Wiki is not up to date for version 3.3.0. #2896

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Systems: Re-enable the modify_interface call #2921

Systems: Re-enable the modify_interface call #2921

SchoolGuy commented Jan 28, 2022 •

edited

Loading

SchoolGuy commented Jan 28, 2022

holmesb commented Jan 28, 2022

SchoolGuy commented Jan 28, 2022

holmesb commented Jan 28, 2022

SchoolGuy commented Jan 28, 2022

codecov bot commented Jan 28, 2022 •

edited

Loading

SchoolGuy commented Jan 28, 2022

holmesb commented Jan 28, 2022

SchoolGuy commented Jan 28, 2022

holmesb commented Jan 29, 2022 •

edited

Loading

SchoolGuy commented Jan 29, 2022

SchoolGuy commented Jan 30, 2022

SchoolGuy commented Jan 31, 2022

SchoolGuy commented Feb 1, 2022

holmesb commented Feb 1, 2022 •

edited

Loading

SchoolGuy commented Feb 1, 2022

holmesb commented Feb 1, 2022

SchoolGuy commented Feb 3, 2022

Systems: Re-enable the modify_interface call #2921

Systems: Re-enable the modify_interface call #2921

Conversation

SchoolGuy commented Jan 28, 2022 • edited Loading

SchoolGuy commented Jan 28, 2022

holmesb commented Jan 28, 2022

SchoolGuy commented Jan 28, 2022

holmesb commented Jan 28, 2022

SchoolGuy commented Jan 28, 2022

codecov bot commented Jan 28, 2022 • edited Loading

Codecov Report

SchoolGuy commented Jan 28, 2022

holmesb commented Jan 28, 2022

SchoolGuy commented Jan 28, 2022

holmesb commented Jan 29, 2022 • edited Loading

SchoolGuy commented Jan 29, 2022

SchoolGuy commented Jan 30, 2022

SchoolGuy commented Jan 31, 2022

SchoolGuy commented Feb 1, 2022

holmesb commented Feb 1, 2022 • edited Loading

SchoolGuy commented Feb 1, 2022

holmesb commented Feb 1, 2022

SchoolGuy commented Feb 3, 2022

SchoolGuy commented Jan 28, 2022 •

edited

Loading

codecov bot commented Jan 28, 2022 •

edited

Loading

holmesb commented Jan 29, 2022 •

edited

Loading

holmesb commented Feb 1, 2022 •

edited

Loading