{"action":"created","comment":{"url":"https://api.github.com/repos/puppetlabs/puppet/pulls/comments/205401569","pull_request_review_id":140661393,"id":205401569,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIwNTQwMTU2OQ==","diff_hunk":"@@ -2,117 +2,139 @@\n \n # Solaris 10 SMF-style services.\n Puppet::Type.type(:service).provide :smf, :parent => :base do\n+  # TODO: Update the documentation with things that you find.\n+  # E.g. About specifying fmri explicitly in the manifest.\n   desc <<-EOT\n     Support for Sun's new Service Management Framework.\n \n-    Starting a service is effectively equivalent to enabling it, so there is\n-    only support for starting and stopping services, which also enables and\n-    disables them, respectively.\n-\n     By specifying `manifest => \"/path/to/service.xml\"`, the SMF manifest will\n     be imported if it does not exist.\n-\n   EOT\n \n   defaultfor :osfamily => :solaris\n \n   confine :osfamily => :solaris\n \n-  commands :adm => \"/usr/sbin/svcadm\", :svcs => \"/usr/bin/svcs\"\n-  commands :svccfg => \"/usr/sbin/svccfg\"\n+  commands :adm => \"/usr/sbin/svcadm\",\n+           :svcs => \"/usr/bin/svcs\",\n+           :svccfg => \"/usr/sbin/svccfg\"\n \n   has_feature :refreshable\n \n-  def setupservice\n-      if resource[:manifest]\n-        begin\n-          svcs(\"-l\", @resource[:name])\n-        rescue Puppet::ExecutionFailure\n-          Puppet.notice \"Importing #{@resource[:manifest]} for #{@resource[:name]}\"\n-          svccfg :import, resource[:manifest]\n-        end\n-      end\n+  def self.instances\n+   service_instances = svcs(\"-H\", \"-o\", \"state,fmri\" ).split(\"\\n\")\n+\n+   # Puppet does not manage services in the legacy_run state, so filter those out.\n+   service_instances.reject! { |line| line =~ /^legacy_run/ }\n+   \n+   service_instances.collect! do |line|\n+     _state, fmri = line.split(/\\s+/)\n+     new({:name => fmri})\n+   end\n+\n+   service_instances\n+  end\n+\n+  def initialize(*args)\n+    super(*args)\n+\n+    # This hash contains the properties we need to sync. in our flush method. We use a separate hash\n+    # instead of @properties_hash to simplify its implementation, since the latter stores ALL of the\n+    # specified properties, not just the ones that need to be syncd.\n+    @properties_to_sync = {}","path":"lib/puppet/provider/service/smf.rb","position":59,"original_position":59,"commit_id":"bbd6fd36f367085a14964d191963c05dcc3a6ab7","original_commit_id":"bbd6fd36f367085a14964d191963c05dcc3a6ab7","user":{"login":"ekinanp","id":29735501,"node_id":"MDQ6VXNlcjI5NzM1NTAx","avatar_url":"https://avatars1.githubusercontent.com/u/29735501?v=4","gravatar_id":"","url":"https://api.github.com/users/ekinanp","html_url":"https://github.com/ekinanp","followers_url":"https://api.github.com/users/ekinanp/followers","following_url":"https://api.github.com/users/ekinanp/following{/other_user}","gists_url":"https://api.github.com/users/ekinanp/gists{/gist_id}","starred_url":"https://api.github.com/users/ekinanp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ekinanp/subscriptions","organizations_url":"https://api.github.com/users/ekinanp/orgs","repos_url":"https://api.github.com/users/ekinanp/repos","events_url":"https://api.github.com/users/ekinanp/events{/privacy}","received_events_url":"https://api.github.com/users/ekinanp/received_events","type":"User","site_admin":false},"body":"Does this seem reasonable? `properties_hash` seemed a little overkill and it seems like not using it would be not be harmful.","created_at":"2018-07-26T10:07:05Z","updated_at":"2018-07-26T10:07:05Z","html_url":"https://github.com/puppetlabs/puppet/pull/6936#discussion_r205401569","pull_request_url":"https://api.github.com/repos/puppetlabs/puppet/pulls/6936","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/comments/205401569"},"html":{"href":"https://github.com/puppetlabs/puppet/pull/6936#discussion_r205401569"},"pull_request":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/6936"}}},"pull_request":{"url":"https://api.github.com/repos/puppetlabs/puppet/pulls/6936","id":202979714,"node_id":"MDExOlB1bGxSZXF1ZXN0MjAyOTc5NzE0","html_url":"https://github.com/puppetlabs/puppet/pull/6936","diff_url":"https://github.com/puppetlabs/puppet/pull/6936.diff","patch_url":"https://github.com/puppetlabs/puppet/pull/6936.patch","issue_url":"https://api.github.com/repos/puppetlabs/puppet/issues/6936","number":6936,"state":"open","locked":false,"title":" (PUP-8957) Fix SMF provider to properly implement enableable semantics","user":{"login":"ekinanp","id":29735501,"node_id":"MDQ6VXNlcjI5NzM1NTAx","avatar_url":"https://avatars1.githubusercontent.com/u/29735501?v=4","gravatar_id":"","url":"https://api.github.com/users/ekinanp","html_url":"https://github.com/ekinanp","followers_url":"https://api.github.com/users/ekinanp/followers","following_url":"https://api.github.com/users/ekinanp/following{/other_user}","gists_url":"https://api.github.com/users/ekinanp/gists{/gist_id}","starred_url":"https://api.github.com/users/ekinanp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ekinanp/subscriptions","organizations_url":"https://api.github.com/users/ekinanp/orgs","repos_url":"https://api.github.com/users/ekinanp/repos","events_url":"https://api.github.com/users/ekinanp/events{/privacy}","received_events_url":"https://api.github.com/users/ekinanp/received_events","type":"User","site_admin":false},"body":"Previously, the SMF provider had the same semantics for enable as it did\r\nfor ensure. Specifically, #enable and #disable delegated to #start and #stop.\r\nThis is completely inconsistent with the enableable semantics for service\r\nproviders, semantics which other Unix-based service providers like systemd\r\nimplement. For example, if you had a SMF-based service resource with\r\nenable => true and ensure => false, the net effect of the operation would\r\nbe a disabled service that would remain disabled when rebooted, which is\r\nincorrect and inconsistent. The correct and consistent effect should be\r\na disabled service that would start up when rebooted.\r\n\r\nEssentially, enableable semantics requires that enable and ensure be\r\nindependent operations where enable handles whether the service starts\r\nor stops at boot time, while ensure handles whether the service starts\r\nor stops in the current running instance. The previous SMF provider was\r\nconflating the two properties together so that enable and ensure were\r\nno longer independent operations.\r\n\r\nThis commit fixes that. It rewrites core parts of the SMF provider to\r\nnow conform to enableable semantics and also fixes some possible issues\r\nwith it. The key takeaways here are:\r\n\r\n* Service commands now use the service FMRI instead of the resource\r\nname. This is the unique resource identifier for the particular\r\ninstance. Previously, we'd pass in the resource name instead which would\r\nmatch a pattern corresponding to multiple service instances. We only\r\nwant to manage one service instance in a single service resource. The code now\r\nfails if more than one FMRI exists for a given service resource name.\r\n\r\n* We now sync. the enable and ensure properties of the service resource in a single\r\ntransaction, specifically inside the #flush method. This is due to the dependency\r\nbetween the ensure and enable properties caused by how SMF works under the hood.\r\n\r\nAcceptance tests for this new behavior have been added. The existing\r\nnonexistent service tests on the AIX service provider have been\r\nrefactored to a common helper function that both the SMF and AIX\r\nprovider tests can call on. This helper function should provide a clean\r\ninterface for nonexistent service tests that may be used for other\r\nservice providers in the future.","created_at":"2018-07-21T00:00:53Z","updated_at":"2018-07-26T10:07:05Z","closed_at":null,"merged_at":null,"merge_commit_sha":"cd17fb5f86103894326b51fbf136d7f0ff3022a9","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":919805083,"node_id":"MDU6TGFiZWw5MTk4MDUwODM=","url":"https://api.github.com/repos/puppetlabs/puppet/labels/Do%20Not%20Merge","name":"Do Not Merge","color":"d93f0b","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/puppetlabs/puppet/pulls/6936/commits","review_comments_url":"https://api.github.com/repos/puppetlabs/puppet/pulls/6936/comments","review_comment_url":"https://api.github.com/repos/puppetlabs/puppet/pulls/comments{/number}","comments_url":"https://api.github.com/repos/puppetlabs/puppet/issues/6936/comments","statuses_url":"https://api.github.com/repos/puppetlabs/puppet/statuses/bbd6fd36f367085a14964d191963c05dcc3a6ab7","head":{"label":"ekinanp:PUP-8957","ref":"PUP-8957","sha":"bbd6fd36f367085a14964d191963c05dcc3a6ab7","user":{"login":"ekinanp","id":29735501,"node_id":"MDQ6VXNlcjI5NzM1NTAx","avatar_url":"https://avatars1.githubusercontent.com/u/29735501?v=4","gravatar_id":"","url":"https://api.github.com/users/ekinanp","html_url":"https://github.com/ekinanp","followers_url":"https://api.github.com/users/ekinanp/followers","following_url":"https://api.github.com/users/ekinanp/following{/other_user}","gists_url":"https://api.github.com/users/ekinanp/gists{/gist_id}","starred_url":"https://api.github.com/users/ekinanp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ekinanp/subscriptions","organizations_url":"https://api.github.com/users/ekinanp/orgs","repos_url":"https://api.github.com/users/ekinanp/repos","events_url":"https://api.github.com/users/ekinanp/events{/privacy}","received_events_url":"https://api.github.com/users/ekinanp/received_events","type":"User","site_admin":false},"repo":{"id":108305884,"node_id":"MDEwOlJlcG9zaXRvcnkxMDgzMDU4ODQ=","name":"puppet","full_name":"ekinanp/puppet","owner":{"login":"ekinanp","id":29735501,"node_id":"MDQ6VXNlcjI5NzM1NTAx","avatar_url":"https://avatars1.githubusercontent.com/u/29735501?v=4","gravatar_id":"","url":"https://api.github.com/users/ekinanp","html_url":"https://github.com/ekinanp","followers_url":"https://api.github.com/users/ekinanp/followers","following_url":"https://api.github.com/users/ekinanp/following{/other_user}","gists_url":"https://api.github.com/users/ekinanp/gists{/gist_id}","starred_url":"https://api.github.com/users/ekinanp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ekinanp/subscriptions","organizations_url":"https://api.github.com/users/ekinanp/orgs","repos_url":"https://api.github.com/users/ekinanp/repos","events_url":"https://api.github.com/users/ekinanp/events{/privacy}","received_events_url":"https://api.github.com/users/ekinanp/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/ekinanp/puppet","description":"Server automation framework and application","fork":true,"url":"https://api.github.com/repos/ekinanp/puppet","forks_url":"https://api.github.com/repos/ekinanp/puppet/forks","keys_url":"https://api.github.com/repos/ekinanp/puppet/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ekinanp/puppet/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ekinanp/puppet/teams","hooks_url":"https://api.github.com/repos/ekinanp/puppet/hooks","issue_events_url":"https://api.github.com/repos/ekinanp/puppet/issues/events{/number}","events_url":"https://api.github.com/repos/ekinanp/puppet/events","assignees_url":"https://api.github.com/repos/ekinanp/puppet/assignees{/user}","branches_url":"https://api.github.com/repos/ekinanp/puppet/branches{/branch}","tags_url":"https://api.github.com/repos/ekinanp/puppet/tags","blobs_url":"https://api.github.com/repos/ekinanp/puppet/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ekinanp/puppet/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ekinanp/puppet/git/refs{/sha}","trees_url":"https://api.github.com/repos/ekinanp/puppet/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ekinanp/puppet/statuses/{sha}","languages_url":"https://api.github.com/repos/ekinanp/puppet/languages","stargazers_url":"https://api.github.com/repos/ekinanp/puppet/stargazers","contributors_url":"https://api.github.com/repos/ekinanp/puppet/contributors","subscribers_url":"https://api.github.com/repos/ekinanp/puppet/subscribers","subscription_url":"https://api.github.com/repos/ekinanp/puppet/subscription","commits_url":"https://api.github.com/repos/ekinanp/puppet/commits{/sha}","git_commits_url":"https://api.github.com/repos/ekinanp/puppet/git/commits{/sha}","comments_url":"https://api.github.com/repos/ekinanp/puppet/comments{/number}","issue_comment_url":"https://api.github.com/repos/ekinanp/puppet/issues/comments{/number}","contents_url":"https://api.github.com/repos/ekinanp/puppet/contents/{+path}","compare_url":"https://api.github.com/repos/ekinanp/puppet/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ekinanp/puppet/merges","archive_url":"https://api.github.com/repos/ekinanp/puppet/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ekinanp/puppet/downloads","issues_url":"https://api.github.com/repos/ekinanp/puppet/issues{/number}","pulls_url":"https://api.github.com/repos/ekinanp/puppet/pulls{/number}","milestones_url":"https://api.github.com/repos/ekinanp/puppet/milestones{/number}","notifications_url":"https://api.github.com/repos/ekinanp/puppet/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ekinanp/puppet/labels{/name}","releases_url":"https://api.github.com/repos/ekinanp/puppet/releases{/id}","deployments_url":"https://api.github.com/repos/ekinanp/puppet/deployments","created_at":"2017-10-25T17:57:54Z","updated_at":"2018-04-20T17:52:55Z","pushed_at":"2018-07-26T09:54:05Z","git_url":"git://github.com/ekinanp/puppet.git","ssh_url":"git@github.com:ekinanp/puppet.git","clone_url":"https://github.com/ekinanp/puppet.git","svn_url":"https://github.com/ekinanp/puppet","homepage":"https://puppet.com/puppet/what-is-puppet","size":65585,"stargazers_count":0,"watchers_count":0,"language":"Ruby","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":2,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":0,"open_issues":2,"watchers":0,"default_branch":"master"}},"base":{"label":"puppetlabs:5.5.x","ref":"5.5.x","sha":"39a17cf7e38d9d48bf42eab2ced135a3d8533325","user":{"login":"puppetlabs","id":234268,"node_id":"MDEyOk9yZ2FuaXphdGlvbjIzNDI2OA==","avatar_url":"https://avatars0.githubusercontent.com/u/234268?v=4","gravatar_id":"","url":"https://api.github.com/users/puppetlabs","html_url":"https://github.com/puppetlabs","followers_url":"https://api.github.com/users/puppetlabs/followers","following_url":"https://api.github.com/users/puppetlabs/following{/other_user}","gists_url":"https://api.github.com/users/puppetlabs/gists{/gist_id}","starred_url":"https://api.github.com/users/puppetlabs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/puppetlabs/subscriptions","organizations_url":"https://api.github.com/users/puppetlabs/orgs","repos_url":"https://api.github.com/users/puppetlabs/repos","events_url":"https://api.github.com/users/puppetlabs/events{/privacy}","received_events_url":"https://api.github.com/users/puppetlabs/received_events","type":"Organization","site_admin":false},"repo":{"id":910744,"node_id":"MDEwOlJlcG9zaXRvcnk5MTA3NDQ=","name":"puppet","full_name":"puppetlabs/puppet","owner":{"login":"puppetlabs","id":234268,"node_id":"MDEyOk9yZ2FuaXphdGlvbjIzNDI2OA==","avatar_url":"https://avatars0.githubusercontent.com/u/234268?v=4","gravatar_id":"","url":"https://api.github.com/users/puppetlabs","html_url":"https://github.com/puppetlabs","followers_url":"https://api.github.com/users/puppetlabs/followers","following_url":"https://api.github.com/users/puppetlabs/following{/other_user}","gists_url":"https://api.github.com/users/puppetlabs/gists{/gist_id}","starred_url":"https://api.github.com/users/puppetlabs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/puppetlabs/subscriptions","organizations_url":"https://api.github.com/users/puppetlabs/orgs","repos_url":"https://api.github.com/users/puppetlabs/repos","events_url":"https://api.github.com/users/puppetlabs/events{/privacy}","received_events_url":"https://api.github.com/users/puppetlabs/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/puppetlabs/puppet","description":"Server automation framework and application","fork":false,"url":"https://api.github.com/repos/puppetlabs/puppet","forks_url":"https://api.github.com/repos/puppetlabs/puppet/forks","keys_url":"https://api.github.com/repos/puppetlabs/puppet/keys{/key_id}","collaborators_url":"https://api.github.com/repos/puppetlabs/puppet/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/puppetlabs/puppet/teams","hooks_url":"https://api.github.com/repos/puppetlabs/puppet/hooks","issue_events_url":"https://api.github.com/repos/puppetlabs/puppet/issues/events{/number}","events_url":"https://api.github.com/repos/puppetlabs/puppet/events","assignees_url":"https://api.github.com/repos/puppetlabs/puppet/assignees{/user}","branches_url":"https://api.github.com/repos/puppetlabs/puppet/branches{/branch}","tags_url":"https://api.github.com/repos/puppetlabs/puppet/tags","blobs_url":"https://api.github.com/repos/puppetlabs/puppet/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/puppetlabs/puppet/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/puppetlabs/puppet/git/refs{/sha}","trees_url":"https://api.github.com/repos/puppetlabs/puppet/git/trees{/sha}","statuses_url":"https://api.github.com/repos/puppetlabs/puppet/statuses/{sha}","languages_url":"https://api.github.com/repos/puppetlabs/puppet/languages","stargazers_url":"https://api.github.com/repos/puppetlabs/puppet/stargazers","contributors_url":"https://api.github.com/repos/puppetlabs/puppet/contributors","subscribers_url":"https://api.github.com/repos/puppetlabs/puppet/subscribers","subscription_url":"https://api.github.com/repos/puppetlabs/puppet/subscription","commits_url":"https://api.github.com/repos/puppetlabs/puppet/commits{/sha}","git_commits_url":"https://api.github.com/repos/puppetlabs/puppet/git/commits{/sha}","comments_url":"https://api.github.com/repos/puppetlabs/puppet/comments{/number}","issue_comment_url":"https://api.github.com/repos/puppetlabs/puppet/issues/comments{/number}","contents_url":"https://api.github.com/repos/puppetlabs/puppet/contents/{+path}","compare_url":"https://api.github.com/repos/puppetlabs/puppet/compare/{base}...{head}","merges_url":"https://api.github.com/repos/puppetlabs/puppet/merges","archive_url":"https://api.github.com/repos/puppetlabs/puppet/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/puppetlabs/puppet/downloads","issues_url":"https://api.github.com/repos/puppetlabs/puppet/issues{/number}","pulls_url":"https://api.github.com/repos/puppetlabs/puppet/pulls{/number}","milestones_url":"https://api.github.com/repos/puppetlabs/puppet/milestones{/number}","notifications_url":"https://api.github.com/repos/puppetlabs/puppet/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/puppetlabs/puppet/labels{/name}","releases_url":"https://api.github.com/repos/puppetlabs/puppet/releases{/id}","deployments_url":"https://api.github.com/repos/puppetlabs/puppet/deployments","created_at":"2010-09-14T19:26:44Z","updated_at":"2018-07-26T09:16:03Z","pushed_at":"2018-07-26T09:54:07Z","git_url":"git://github.com/puppetlabs/puppet.git","ssh_url":"git@github.com:puppetlabs/puppet.git","clone_url":"https://github.com/puppetlabs/puppet.git","svn_url":"https://github.com/puppetlabs/puppet","homepage":"https://puppet.com/puppet/what-is-puppet","size":67122,"stargazers_count":5049,"watchers_count":5049,"language":"Ruby","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":2021,"mirror_url":null,"archived":false,"open_issues_count":46,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":2021,"open_issues":46,"watchers":5049,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/6936"},"html":{"href":"https://github.com/puppetlabs/puppet/pull/6936"},"issue":{"href":"https://api.github.com/repos/puppetlabs/puppet/issues/6936"},"comments":{"href":"https://api.github.com/repos/puppetlabs/puppet/issues/6936/comments"},"review_comments":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/6936/comments"},"review_comment":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/6936/commits"},"statuses":{"href":"https://api.github.com/repos/puppetlabs/puppet/statuses/bbd6fd36f367085a14964d191963c05dcc3a6ab7"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"ekinanp"}}