Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature "expanded custom facts" #113

Open
wants to merge 10 commits into
base: production
Choose a base branch
from

Conversation

MemberIT
Copy link

@MemberIT MemberIT commented Aug 3, 2016

New feature "expanded custom facts"
Puppet code:

  $title_facters = {
    my_fact => {
      ensure => present,
      value  =>
        ['my_val0', 'my_val1'],
    },
    my_fact2 => {
      ensure => present,
      value  => {
        key1 => [
          'val11',
          'val12',
        ],
        key2 => {
          key21 => 'val211',
          key22 => [
            'val221',
            'val222',
          ],
        },
      },
    },
  }
  create_resources('::puppet::fact', $title_facters)

facters:

# facter -v
3.3.0 (commit fc9282a2d426f7c8a742fd5b23c4cb06852eee20)
# facter my_fact
[
  "my_val0",
  "my_val1"
]
# facter my_fact2
{
  key1 => [
    "val11",
    "val12"
  ],
  key2 => {
    key21 => "val211",
    key22 => [
      "val221",
      "val222"
    ]
  }
}
# facter my_fact2.key1
[
  "val11",
  "val12"
]
# facter my_fact2.key2
{
  key21 => "val211",
  key22 => [
    "val221",
    "val222"
  ]
}

Gemfile Outdated
gem "rspec-core", '3.1.7', :require => false
gem "rspec-puppet", '2.3.2', :require => false
gem "rspec-core", '3.5.1', :require => false
gem "rspec-puppet", '2.4.0', :require => false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you upgrading these?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's only gems update.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realise that. Why are you wanting to upgrade them?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me is more information in build travis-ci log.
Before. After.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does look more useful.
If the rest of the tests pass as is then it's good for me.

@rendhalver
Copy link
Member

All of your examples are with either an array or a hash.
You didn't actually provide me an example of a string.
I want to see an example where the fact is a string with various contents.
I particularly want to see what it does with a string with multiple words.
My current code works setting up facts with any kind of string so for me to accept your code it will need to do that as well as arrays and hashes.

@MemberIT
Copy link
Author

Sorry... Now I understand you.

  • For Puppet 4:
# puppet -V
4.5.3
# facter -v
3.3.0 (commit fc9282a2d426f7c8a742fd5b23c4cb06852eee20)
# cat /opt/puppetlabs/facter/facts.d/test.yaml
---
  key4: asdas sadas
  key5: 1 asdq
  key6: 11
# facter key4
asdas sadas
# facter key5
1 asdq
# facter key6
11

Check in code:

  $test = $::key4
  validate_string($test, $::key4, $::key5)
  validate_numeric($::key6)
  notify{"Strings ${test}, ${::key4}, ${::key5}":}

Run code:

Notice: Strings asdas sadas, asdas sadas, 1 asdq
Notice: /Stage[main]/Profiles::Puppetserver/Notify[Strings asdas sadas, asdas sadas, 1 asdq]/message: defined 'message' as 'Strings asdas sadas, asdas sadas, 1 asdq'

validate is good.

  • For Puppet 3:
# puppet -V
3.7.5
# facter -v                                                                                                                                                                                                                           
2.4.4                                                                                                                                                                                                                                 
# cat /etc/facter/facts.d/test.yaml                                                                                                                                                                                                   
---                                                                                                                                                                                                                                   
  key4: asdas sadas                                                                                                                                                                                                                   
  key5: 1 asdq                                                                                                                                                                                                                        
  key6: 11                                                                                                                                                                                                                            
# facter key4                                                                                                                                                                                                                         
asdas sadas                                                                                                                                                                                                                           
# facter key5                                                                                                                                                                                                                         
1 asdq                                                                                                                                                                                                                                
# facter key6                                                                                                                                                                                                                         
11

Check in code:

  $test = $::key4
  validate_string($test, $::key4, $::key5)
  validate_numeric($::key6)
  notify{"Strings ${test}, ${::key4}, ${::key5}":}

Run code:

Notice: /Stage[main]/Main/Notify[Strings , asdas sadas, 1 asdq]/message: current_value absent, should be Strings , asdas sadas, 1 asdq (noop)

validate is good.

@rendhalver
Copy link
Member

So how do you generate those fact files with your code?
This is what I want to see.
I need to know that your code will produce those files and that the files it generates work with facter.
Your last comment is only showing me the later.

@MemberIT
Copy link
Author

Example only for Puppet4:
puppet code:

  $title_facters = {
    key4 => {
      ensure => present,
      value => 'asdas sadas',
    },
    key5 => {
      ensure => present,
      value => '1 asdq',
    },
    key6 => {
      ensure => present,
      value => 1,
    },
    'asdas asdas' => {
      ensure => present,
      value =>'ok',
    },
  }
  # Disable validate
  #$test = $::key4
  #validate_string($test, $::key4, $::key5)
  #validate_numeric($::key6)
  #notify{"Strings ${test}, ${::key4}, ${::key5}":}

  create_resources('::puppet::fact', $title_facters)

Puppet run:

Notice: /Stage[main]/Profiles::Puppetserver/Puppet::Fact[key4]/File[/opt/puppetlabs/facter/facts.d/key4.yaml]/ensure: defined content as '{md5}6b6829b65e4da8976797c5ca1342ff3d'
Notice: /Stage[main]/Profiles::Puppetserver/Puppet::Fact[key5]/File[/opt/puppetlabs/facter/facts.d/key5.yaml]/ensure: defined content as '{md5}b1498fee11fae28af157553032b4d649'
Notice: /Stage[main]/Profiles::Puppetserver/Puppet::Fact[key6]/File[/opt/puppetlabs/facter/facts.d/key6.yaml]/ensure: defined content as '{md5}dbb0e9e6182e80982c7fdd325e95d402'
Notice: /Stage[main]/Profiles::Puppetserver/Puppet::Fact[asdas asdas]/File[/opt/puppetlabs/facter/facts.d/asdas asdas.yaml]/ensure: defined content as '{md5}cf4ca7ad3e235551162ac0e6353cd698'

facter check:

# facter -v
3.3.0 (commit fc9282a2d426f7c8a742fd5b23c4cb06852eee20)
# facter key4
asdas sadas
# facter key5
1 asdq
# facter key6
1
# facter 'asdas asdas'
ok

puppet code:

  $title_facters = {
    key4 => {
      ensure => present,
      value => 'asdas sadas',
    },
    key5 => {
      ensure => present,
      value => '1 asdq',
    },
    key6 => {
      ensure => present,
      value => 1,
    },
    'asdas asdas' => {
      ensure => present,
      value =>'ok',
    },
  }
  # Enable validate
  $test = $::key4
  validate_string($test, $::key4, $::key5)
  validate_numeric($::key6)
  notify{"Strings ${test}, ${::key4}, ${::key5}":}

  create_resources('::puppet::fact', $title_facters)

Puppet run:

Notice: Strings asdas sadas, asdas sadas, 1 asdq
Notice: /Stage[main]/Profiles::Puppetserver/Notify[Strings asdas sadas, asdas sadas, 1 asdq]/message: defined 'message' as 'Strings asdas sadas, asdas sadas, 1 asdq'

I think is not good file name:

# ls -l /opt/puppetlabs/facter/facts.d/ | grep asdas
-rw-r----- 1 root root  47 авг 11 17:24 asdas asdas.yaml

I update branch.

@rendhalver
Copy link
Member

@MemberIT Please remove your edits to Gemfile and Gemfile.lock and rebase.
In have adjusted those since you opened this PR.

@MemberIT MemberIT force-pushed the feature/expanded_custom_facts branch 6 times, most recently from 610a849 to cb0cb9a Compare April 4, 2017 14:03
@MemberIT MemberIT force-pushed the feature/expanded_custom_facts branch 2 times, most recently from ff2b3ae to bf7d3ba Compare April 4, 2017 14:20
@MemberIT MemberIT force-pushed the feature/expanded_custom_facts branch from bf7d3ba to 773b037 Compare April 5, 2017 08:26
@MemberIT
Copy link
Author

MemberIT commented Apr 5, 2017

@rendhalver done

@rendhalver
Copy link
Member

Thanks @MemberIT. I will have a look at this locally soon to see how it works.
I am mostly looking for API breakage and so it still works under puppet 3 for now.
Thanks for the contribution.

@MemberIT MemberIT force-pushed the feature/expanded_custom_facts branch from be43cc2 to e864256 Compare May 19, 2017 08:57
@MemberIT MemberIT force-pushed the feature/expanded_custom_facts branch from e864256 to 5d4054e Compare May 19, 2017 09:12
@toepi
Copy link
Contributor

toepi commented May 31, 2017

Great improvement for facts. Why still puppet3 - have learn is no longer supported by puppetlabs - more and more are puppet4 only..

@rendhalver
Copy link
Member

rendhalver commented Jun 1, 2017

That validation is still useful.
Has that option been removed from the file resource?

@MemberIT
Copy link
Author

MemberIT commented Jun 1, 2017

@rendhalver That validation not needed. Code in templates of custom facters
templates/fact.yaml.erb:

<%- require 'yaml' -%>
<%= YAML.dump(@facter_data) %>

templates/local_facts.yaml.erb:

<%- require 'yaml' -%>
<%= YAML.dump(@custom_facts) %>

'yaml' is part of ruby stdlib in all versions:
https://ruby-doc.org/stdlib-1.8.7/libdoc/yaml/rdoc/YAML.html
https://ruby-doc.org/stdlib-1.9.3/libdoc/yaml/rdoc/YAML.html
...
https://ruby-doc.org/stdlib-2.4.1/libdoc/yaml/rdoc/YAML.html
Method 'dump' suported in all ruby versions.
Validation yaml content not needed, because content created with yaml library.

@MemberIT
Copy link
Author

MemberIT commented Jun 1, 2017

@toepi Current version of code correctly work on Puppet 3 and Puppet 4. I would not change the code of manifests in pullrequest, if module worked only Puppet 4.

@toepi
Copy link
Contributor

toepi commented Jun 1, 2017

@MemberIT: I like It and use it since yesterday in my lab. And have saw how can I solve another of my template issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants