goss-org / goss

Quick and Easy server testing/validation
https://goss.rocks
Apache License 2.0
5.6k stars 472 forks source link

Discuss feature #570

Closed aelsabbahy closed 4 years ago

aelsabbahy commented 4 years ago

Describe the feature:

This is in regards to the following two PRs. Since they were created without an issue first. This issue is to discuss the intent of them and go through the approval process. As documented in the contribution guide.

https://github.com/aelsabbahy/goss/blob/master/.github/CONTRIBUTING.md

Discussing the following two PRs:

Describe the solution you'd like

Describe alternatives you've considered

aelsabbahy commented 4 years ago

Can you describe the use-case for both #558 and #561

One use-case I can think of for #558 is creating virtual interfaces and explicitly setting the MAC and validating them. Not sure if there was another use-case intended since isVirtual seems to be a separate flag.

I'm not sure I understand #561.

@pedroMMM , going to loop you into this chat since you commented on both PRs

weakiwi commented 4 years ago

Hello @aelsabbahy

For #558, I just try to expose more attribute for an interface. As for use-case, I think https://serverfault.com/questions/943363/how-to-assure-the-same-mac-address-is-used-to-bond-interfaces-using-anaconda-kic can be an example. Bond0 interface must be virtual devicde and the mac address of bond interfaces should be same

THX

pedroMMM commented 4 years ago

For #558 out of itself, most users will not have a good use case. The ones that need to concern themselves with MAC aren't many.

Direction @weakiwi is going by adding support for ghw is great! Making Goss more hardware friendly is awesome for those of us that work lower in the stack.

I could have used #561 features when I was doing migrations of our EC2 fleets to Nitro based instances. The device bootstrapping was a pain to get right and having Goss at the time testing the device type would have been helpful.

These features will not help the majority of the users but for those that it targets, it will be a significant quality of life upgrade.

aelsabbahy commented 4 years ago

558 use-case makes sense to me since:

  1. it's a lightweight change to an already existing test (interface)
  2. While the virtual/mac spoofing attribute isn't that universally used, it's a minor change to a common test (interface)

561 and @donmstewart comment above seem like scope creep for goss.

  • Goss is focused on the 20% of the 80/20 rule. In other words, Goss focuses on the 20% of features that cover the core aspects of OS testing and benefit 80% of users.
  • Goss provides a generic command runner to allow users to cover more nuanced test cases.

-- https://github.com/aelsabbahy/goss/blob/master/.github/CONTRIBUTING.md#feature-requests

There's an almost a never-ending list of tests that could be added to goss if that rule isn't applied.

I would be more interested in seeing if there are ways to improve the command test or a new way for users to hook into (or extend) goss to allow users to write custom test modules for these less-common scenarios.

Basically, I don't see goss trying to be a poor implementation of other hardware probing tools (hwinfo, dmidecode, lshw, inxi etc..). However, it would be nice if it's flexible enough to easily leverage them.

pedroMMM commented 4 years ago

@aelsabbahy we should decouple #561, and @donmstewart use case, since the latter can still be implemented just by having #558.

This enables us to just focus on #561, which is just testing devices. A lot of us have to deal with devices, from the person that manages the bare metal, to the person that manages the VMs, even to the one creating the containers that run on top of it all. We all might need to check that a device is mounted if it's just to check if the sidecar container was loaded correctly. I would even go forwarder and say that we should have a partition resource!

I understand that in OSS a no is temporary and a yes is forever, but this set of features could make a difference.

Side note though, I see much less use case on #558 than on #561.

aelsabbahy commented 4 years ago

I'm not sure I follow, goss already has a mount resource. Can you give me a code based example of what you mean with regards to testing a block device?

I could be wrong, but I don't think hardware checks is something even heavier weight testing tools (serverspec, inspec) take on and goss was definitely intended to be a much simpler/lighter tool.

Guess what I'm saying is, I'm having a hard time seeing this as falling in this category:

Goss is focused on the 20% of the 80/20 rule. In other words, Goss focuses on the 20% of features that cover the core aspects of OS testing and benefit 80% of users.

I understand that in OSS a no is temporary and a yes is forever, but this set of features could make a difference.

The yes being forever is why I'm leaning against #561 currently. It seems goss would inherit a new domain (hardware testing) and all the bugs that come with ghw and ultimately do a crappier job than a dedicated tool would.

pedroMMM commented 4 years ago

I'm not sure I follow, goss already has a mount resource. Can you give me a code based example of what you mean with regards to testing a block device?

I completely forgot about the mount resource! This invalidates a lot of the value I was seeing here, especially for the short term.

I will show myself out

I could be wrong, but I don't think hardware checks is something even heavier weight testing tools (serverspec, inspec) take on and goss was definitely intended to be a much simpler/lighter tool. ... a new way for users to hook into (or extend) goss to allow users to write custom test modules for these less-common scenarios.

Maybe this is the solution here! Goss will not have any Hardware testing now but depending on the community we could pursue a plugin system.

Which turns the no from firm to we need to architecture the correct solution here.

The yes being forever is why I'm leaning against #561 currently. It seems goss would inherit a new domain (hardware testing) and all the bugs that come with ghw and ultimately do a crappier job than a dedicated tool would.

Then you might not want ghw in the codebase at all, which means we would need to re-evaluate #558.

weakiwi commented 4 years ago

Hello @pedroMMM and @aelsabbahy For the implement of plugins, I have an idea. I use python to explain the code logic. We can talk about the implement and then I can create a new PR for this.

plugin:
  /tmp/test.sh:
      eth0:
        is-virtual: false
        is-bond: false
import yaml
import pprint
import json
import get_command_output
with open("test.yml") as f:
        data = yaml.load(f)

def get_options(executable_file):
        return get_command_output("%s options" % executable_file).strip().split(",")

def get_output_by_option(executable_file, option, target):
        return get_command_output("%s %s %s" % (executable_file, option, target)).strip()

for executable_file in  data["plugin"]:
        for execute_target in data["plugin"][executable_file]:
                for k,v in data["plugin"][executable_file][execute_target].items():
                        print "k=%s, v=%s, result=%s" % (k,v,json.loads(get_output_by_option(executable_file, k, execute_target)))
                        print(get_output_by_option(executable_file, k, execute_target) == v)
check_virtual() {
        echo "false"
}

check_bond() {
        echo "false"
}

case $1 in
        is-virtual)
                check_virtual $2
                exit 0
                ;;
        is-bond)
                check_bond $3
                exit 0
                ;;
        options)
                echo "is-virtual, is-bond"
                exit 0
                ;;
        *)
                echo "Usage: $0 {options|is-virtual|is-bond}"
                exit 0
                ;;
esac
weakiwi commented 4 years ago

Hello @pedroMMM and @aelsabbahy For the implement of plugins, I have an idea. I use python to explain the code logic. We can talk about the implement and then I can create a new PR for this.

plugin:
  /tmp/test.sh:
      eth0:
        is-virtual: false
        is-bond: false
import yaml
import pprint
import json
import get_command_output
with open("test.yml") as f:
        data = yaml.load(f)

def get_options(executable_file):
        return get_command_output("%s options" % executable_file).strip().split(",")

def get_output_by_option(executable_file, option, target):
        return get_command_output("%s %s %s" % (executable_file, option, target)).strip()

for executable_file in  data["plugin"]:
        for execute_target in data["plugin"][executable_file]:
                for k,v in data["plugin"][executable_file][execute_target].items():
                        print "k=%s, v=%s, result=%s" % (k,v,json.loads(get_output_by_option(executable_file, k, execute_target)))
                        print(get_output_by_option(executable_file, k, execute_target) == v)
check_virtual() {
        echo "false"
}

check_bond() {
        echo "false"
}

case $1 in
        is-virtual)
                check_virtual $2
                exit 0
                ;;
        is-bond)
                check_bond $3
                exit 0
                ;;
        options)
                echo "is-virtual, is-bond"
                exit 0
                ;;
        *)
                echo "Usage: $0 {options|is-virtual|is-bond}"
                exit 0
                ;;
esac

For this implement, I think the plugin should follow these rules

  1. need option to output the supported test method for this plugin. Then our program can use the output to extend the structure.
  2. the test method for plugin should be ./test.sh name-of-test test-target. And the format of output is TBD

If you have anything to add, please reply.

THX

aelsabbahy commented 4 years ago

@weakiwi can you start a new feature request with your last post.

I think goss plugin support should be it's own discussion.

Your last post is a great start for that discussion.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

aelsabbahy commented 4 years ago

I think for now the solution will be allowing goss to have better command support, tracking this here: https://github.com/aelsabbahy/goss/issues/578