Bug 254339 - net/cloud-init: fix modifying rc.conf in cc_salt_minion
Summary: net/cloud-init: fix modifying rc.conf in cc_salt_minion
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Brad Davis
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-03-16 17:01 UTC by Brad Davis
Modified: 2022-02-13 22:55 UTC (History)
4 users (show)

See Also:
andrey: maintainer-feedback+

Attachments
patch (1.97 KB, patch)
2021-03-16 17:01 UTC, Brad Davis 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Brad Davis freebsd_committer freebsd_triage 2021-03-16 17:01:57 UTC 
Created attachment 223327 [details]
patch

The rhel update_sysconfig_file() isn't well suited for *BSD style rc.conf so use the set_rc_config_value() instead.

This fixes a bug that prevents the salt module from enabling the salt minion in rc.conf.

Mina: Please upstream if you want.
Comment 1 commit-hook freebsd_committer freebsd_triage 2021-03-17 15:28:56 UTC 
A commit references this bug:

Author: brd
Date: Wed Mar 17 15:28:54 UTC 2021
New revision: 568666
URL: https://svnweb.freebsd.org/changeset/ports/568666

Log:
  net/cloud-init: Fix modifying rc.conf in cc_salt_minion

  PR:		254339
  Approved by:	Andrey Fesenko (maintainer), swills

Changes:
  head/net/cloud-init/Makefile
  head/net/cloud-init/files/
  head/net/cloud-init/files/patch-cloudinit_config_cc__salt__minion.py
Comment 2 Li-Wen Hsu freebsd_committer freebsd_triage 2021-03-18 13:28:49 UTC 
Can this ticket be closed? Or it needs MFH?
Comment 3 Brad Davis freebsd_committer freebsd_triage 2021-03-18 18:15:28 UTC 
MFH is probably a good idea, but mostly I was hoping Mina would confirm upstreaming the fix.
Comment 4 Mina Galić freebsd_triage 2021-03-18 19:21:47 UTC 
Mina is currently very busy, but will bei happily upstream this
Comment 5 Ed Maste freebsd_committer freebsd_triage 2021-11-05 18:15:47 UTC 
It looks like this is unconditional and would break Linux?:

-from cloudinit.distros import rhel_util
+from cloudinit.distros import bsd_utils

Should it instead be this?

diff --git a/cloudinit/config/cc_salt_minion.py b/cloudinit/config/cc_salt_minion.py
index b61876aa..379ea800 100644
--- a/cloudinit/config/cc_salt_minion.py
+++ b/cloudinit/config/cc_salt_minion.py
@@ -46,6 +46,7 @@ specify them with ``pkg_name``, ``service_name`` and ``config_dir``.
 import os
 
 from cloudinit import safeyaml, subp, util
+from cloudinit.distros import bsd_utils
 from cloudinit.distros import rhel_util
 
 
@@ -125,8 +126,7 @@ def handle(name, cfg, cloud, log, _args):
     # we need to have the salt minion service enabled in rc in order to be
     # able to start the service. this does only apply on FreeBSD servers.
     if cloud.distro.osfamily == 'freebsd':
-        rhel_util.update_sysconfig_file(
-            '/etc/rc.conf', {'salt_minion_enable': 'YES'})
+        bsd_utils.set_rc_config_value('salt_minion_enable', 'YES')
 
     # restart salt-minion. 'service' will start even if not started. if it
     # was started, it needs to be restarted for config change.
Comment 6 Mina Galić freebsd_triage 2022-02-04 13:15:40 UTC 
There's a pull request now upstream: https://github.com/canonical/cloud-init/pull/1236

@edmaste, note that your patch would lead to a pylint complaint, because rhel_utils is *not* used in that file.
Comment 7 Mina Galić freebsd_triage 2022-02-08 08:59:50 UTC 
This has now been merged: https://github.com/canonical/cloud-init/pull/1236 !
Comment 8 Brad Davis freebsd_committer freebsd_triage 2022-02-13 22:55:30 UTC 
Thanks Mina! :)