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

Discuss feature #570

Closed
aelsabbahy opened this issue Apr 6, 2020 · 13 comments
Closed

Discuss feature #570

aelsabbahy opened this issue Apr 6, 2020 · 13 comments

Comments

@aelsabbahy
Copy link
Member

aelsabbahy commented Apr 6, 2020

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
Copy link
Member Author

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
Copy link
Contributor

weakiwi commented Apr 6, 2020

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
Copy link
Contributor

pedroMMM commented Apr 6, 2020

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
Copy link
Member Author

aelsabbahy commented Apr 7, 2020

#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
Copy link
Contributor

pedroMMM commented Apr 8, 2020

@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
Copy link
Member Author

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
Copy link
Contributor

pedroMMM commented Apr 8, 2020

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
Copy link
Contributor

weakiwi commented Apr 19, 2020

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
Copy link
Contributor

weakiwi commented Apr 19, 2020

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
Copy link
Member Author

aelsabbahy commented Apr 20, 2020

@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
Copy link

stale bot commented Jul 9, 2020

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.

@stale stale bot added the wontfix label Jul 9, 2020
@aelsabbahy
Copy link
Member Author

I think for now the solution will be allowing goss to have better command support, tracking this here: #578

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

No branches or pull requests

4 participants
@aelsabbahy @weakiwi @pedroMMM and others