(MODULES-3758) Fix: chocolateysource user ensure sync #42
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ferventcoder
changed the title
ferventcoder
force-pushed
the
ticket/master/MODULES-3758-user_empty
branch
2 times, most recently
from
0b23385
to
c2c9b82
Compare
|
Kicked off adhoc for this at https://jenkins-modules.puppetlabs.com/view/3.%20windows%20only/view/ad%20hoc/view/chocolatey/job/forge-windows_puppetlabs-chocolatey_init-manual-parameters_adhoc/23/. It probably won't pass due to the current failure in the pipeline, but hoping that it doesn't present any new issues. |
ferventcoder
changed the title
ferventcoder
force-pushed
the
ticket/master/MODULES-3758-user_empty
branch
from
c2c9b82
to
641d384
Compare
| # act | ||
| step 'Apply manifest a second time' | ||
| apply_manifest(chocolatey_src, :catch_failures => true) do |result| | ||
| assert_not_match(/Chocolateysource\[chocolatey\]\/user\: defined 'user' as ''/, result.stdout, "User was adjusted and should not have been") |
There was a problem hiding this comment.
Fixed this error message. "adjust" -> "adjusted"
ferventcoder
changed the title
During the check for insync, user keeps getting detected for synchronize due to the default value being set to '' as compared to a nil value being returned from the existing items. This can cause errors as it could set a source to enabled, even if the source should be disabled. Instead we should set a default for the existing item so it matches the default in the user property.
ferventcoder
force-pushed
the
ticket/master/MODULES-3758-user_empty
branch
from
641d384
to
44f8bea
Compare
When validating at the type level, typically all parameters/properties has been set. The only instance of this not being true is with `puppet resource`. With puppet resource, values are initially set to their defaults when validation is called. This means location is sometimes empty coming from the resource path. When validation runs, the resource creation has not actually set the properties from the property_hash. This causes the top-level resource validation to fail. The resource is created and passed to provider.initialize, which sets the property_hash differently than when run through the normal manifest. https://github.com/puppetlabs/puppet/blob/5bb93f0c06cca6cd16f693e9dbaf2066d43a7b6a/lib/puppet/provider.rb#L499-L519 This is due to the call to self.instances resulting in initializing the instances prior to setting properties - see https://github.com/puppetlabs/puppet/blob/5bb93f0c06cca6cd16f693e9dbaf2066d43a7b6a/lib/puppet/type.rb#L1179-L1180. Then it calls initialize still not having set the properties, it runs validate - https://github.com/puppetlabs/puppet/blob/5bb93f0c06cca6cd16f693e9dbaf2066d43a7b6a/lib/puppet/type.rb#L2391 This only happens during `puppet resource type` - when running Puppet normally, it does not run into this issue. This seems to be a small issue in how puppet resource works with self.instances and validation at the type level. Look in multiple places to find the value of `location`.
When flushing values, be sure the value of ensure is found. It may not be part of what is changing for the flush - it could be another property. It is imperative to know the value of the ensure, so start with `property_hash[:ensure]` value, then look at `property_flush[:ensure]` if it is available. If ensure is still not found, look at `resource[:ensure]`. When disabling a source, that source must first exist. Disabling a source removes two calls to choco, one on the version and another for adding the source prior to disabling. Unfortunately it doesn't easily allow bringing on a new machine with the same manifests when wanting to disable a source that hasn't priorly existed. This is a very slight edge case. Typically few sources would be disabled, most would outright be removed.
ferventcoder
force-pushed
the
ticket/master/MODULES-3758-user_empty
branch
from
44f8bea
to
edcd94b
Compare
ferventcoder
changed the title
|
👍 verified working |
ThoughtCrhyme
pushed a commit
to ThoughtCrhyme/puppetlabs-chocolatey
that referenced
this pull request
Mar 20, 2017
…5/refactor (QA-2805) Refactor Code into Python Package
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During the check for insync, user keeps getting detected for synchronize
due to the default value being set to '' as compared to a nil value
being returned from the existing items.
This can cause errors as it could set a source to enabled, even if the
source should be disabled. Instead we should set a default for the
existing item so it matches the default in the user property.
Still needs:
Fixes for MODULES-3430 have also been included.