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

Fix interpolation issue in the splunk_login_successful method #205

Closed
wants to merge 17 commits into from

Conversation

haidangwa
Copy link
Contributor

@haidangwa haidangwa commented Mar 15, 2021

Description

  • Fixes Issue #204
  • Fixes an issue with the user-seed.conf file
  • Ensures that splunk is installed prior to anything in the chef-splunk::service recipe executes

Issues Resolved

#204

Check List

  • All tests pass. See TESTING.md for details.
  • New functionality includes testing.
  • New functionality has been documented in the README if applicable.

@ramereth ramereth added the Release: Patch Release to Chef Supermarket as a version patch when merged label Mar 16, 2021
@haidangwa haidangwa force-pushed the gh-204 branch 2 times, most recently from fac3e7f to f301543 Compare May 12, 2021 14:34
@ramereth
Copy link
Contributor

@haidangwa can you please rebase instead of doing a merge commit update to keep the history linear? Also please address the chefspec issues.

@haidangwa
Copy link
Contributor Author

haidangwa commented May 12, 2021 via email

@haidangwa haidangwa force-pushed the gh-204 branch 5 times, most recently from 6283047 to f42b64b Compare May 13, 2021 02:03
@the-label-bot

This comment has been minimized.

@the-label-bot

This comment has been minimized.

@ramereth
Copy link
Contributor

@haidangwa looks like we need to deal with some Chef 17 issues before we can get this merged unfortunately.

@haidangwa
Copy link
Contributor Author

@ramereth I'm running the latest chef-workstation on my laptop and the test for uninstall-forwarder passes in Test Kitchen with Chef 17. I'm not sure why this is failing in Github Actions.

@ramereth
Copy link
Contributor

@haidangwa I can replicate the problem on my end with 17.1.35. I suspect it has something to do with the systemd? helper included the cookbook conflicting with something else. That really should use chef-utils helpers IMO, but that would increase the Chef Client requirement (which we're going to need to do anyway).

@haidangwa
Copy link
Contributor Author

haidangwa commented May 13, 2021

The issue is between Chef Infra Client v17.0.150 and 17.0.242.

All tests pass under v17.0.150, but the uninstall-forwarder tests fail under 17.0.242

@ramereth

@the-label-bot

This comment has been minimized.

@the-label-bot

This comment has been minimized.

1 similar comment
@the-label-bot
Copy link

the-label-bot bot commented May 13, 2021

Kind Prediction: fix
Confidence: 0.999

Provide the bot with feedback using a 👍 or 👎!

@haidangwa
Copy link
Contributor Author

haidangwa commented May 13, 2021

@ramereth I've narrowed this down even further. Using kitchen-dokken, the chef infra clients pass all the way through and including 17.0.194. It breaks between 17.0.194 and 17.0.199. That seems definitive that it is a Chef Infra Client bug.

17.0.194: PASS

CHEF_VERSION=17.0.194 KITCHEN_LOCAL_YAML=kitchen.dokken.yml kitchen converge uninstall-forwarder-amazonlinux-2

17.0.199: FAIL *uninstall-forwarder test suites

CHEF_VERSION=17.0.199 KITCHEN_LOCAL_YAML=kitchen.dokken.yml kitchen converge uninstall-forwarder-amazonlinux-2

based on tags from docker hub

@haidangwa
Copy link
Contributor Author

haidangwa commented May 13, 2021

looks like ruby 3.0.1 was added to the chef omnibus_overrides.rb between v17.0.197 and v17.0.198

diff --git a/omnibus_overrides.rb b/omnibus_overrides.rb
index d20d702d73..32c2cc6e69 100644
--- a/omnibus_overrides.rb
+++ b/omnibus_overrides.rb
@@ -16,7 +16,8 @@ override "ncurses", version: "5.9"
 override "nokogiri", version: "1.11.0"
 override "openssl", version: mac_os_x? ? "1.1.1k" : "1.0.2y"
 override "pkg-config-lite", version: "0.28-1"
-override "ruby", version: "2.7.2"
+override "bundler", version: "2.2.15"
+override "ruby", version: "3.0.1"
 override "ruby-windows-devkit-bash", version: "3.1.23-4-msys-1.0.18"
 override "util-macros", version: "1.19.0"
 override "xproto", version: "7.0.28"

@haidangwa
Copy link
Contributor Author

I think getting this cookbook to be compatible with Chef Infra Client 17 is beyond the scope of this PR. Fixing the changes to be Chef 17 compatible should be in itself a separate Pull Request.

@haidangwa
Copy link
Contributor Author

haidangwa commented May 14, 2021

@ramereth OK. With all that said (above), I am thinking the problem is in the service provider/resource; perhaps the recent changes to the service resource in Chef 17. In my custom resource, splunk_installer, it calls two helper methods, and both helper methods are referencing the node data.

  service 'Splunk' do
    service_name server? ? 'Splunkd' : 'SplunkForwarder'
    action systemd? ? %i(stop disable) : :stop
  end

Note: server? and systemd? are helper methods declared in libraries/helpers.rb and both return true/false after making a comparison using a node attribute.

    def server?
      node['splunk']['is_server'] == true
    end

     def systemd?
       node['init_package'] == 'systemd'
     end

However, the error does not fail for the server? and fails for the systemd?. It is perhaps a bug in the service provider's action argument.

It's also not that the custom resource doesn't have access to the node object, as I confirmed this by adding a log resource right before my custom resource uses the service resource. My log resource did not fail in my test.

  log "init_package is #{node['init_package']}"

  service 'Splunk' do
    service_name server? ? 'Splunkd' : 'SplunkForwarder'
    action systemd? ? %i(stop disable) : :stop
  end

In fact, it worked fine, as demonstrated in my chef run output, below:

    * log[init_package is systemd] action write

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
kitchen.dokken.yml Outdated Show resolved Hide resolved
kitchen.yml Outdated Show resolved Hide resolved
@damacus damacus changed the title Fixes Issue #204 Fixes interpolation issue in the splunk_login_successful method Sep 1, 2021
@damacus damacus changed the title Fixes interpolation issue in the splunk_login_successful method Fix interpolation issue in the splunk_login_successful method Sep 1, 2021
@ramereth
Copy link
Contributor

@haidangwa so it looks like some of the fixes you have in here were done in #215 and conflicts with this. Can you please rebase and resolve the conflicts? Thanks!

haidangwa and others added 12 commits December 2, 2021 15:41
- Fixes an issue with the `user-seed.conf` file
- Ensures that splunk is installed prior to anything in the `chef-splunk::service` recipe executes

Signed-off-by: Dang H. Nguyen <dang.nguyen@disney.com>
- set `unified_mode true` in all custom resources

Signed-off-by: Dang H. Nguyen <dang.nguyen@disney.com>
- Renames inspec files to end with `_test.rb` rather than `_spec.rb` to distinguish from chefspec files

Signed-off-by: Dang H. Nguyen <dang.nguyen@disney.com>
…om Splunk; therefore, this change will use the 8.0.9 RPM instead

Signed-off-by: Dang H. Nguyen <dang.nguyen@disney.com>
… presentation of `ps 1` command

Signed-off-by: Dang H. Nguyen <dang.nguyen@disney.com>
…version dependency to `~> 4.0.0` for unified mode
…n initializing a search head cluster member
@damacus damacus closed this Oct 3, 2023
@damacus
Copy link
Member

damacus commented Oct 3, 2023

Hey @haidangwa can you reopen this PR when you have a clean branch. Thanks!

Looks like some great work here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release: Patch Release to Chef Supermarket as a version patch when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

splunk_login_successful? helper method fails to send auth info to splunkd
4 participants