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

feat: add acceptance test framework #4301

Closed
wants to merge 10 commits into from
Closed

Conversation

fmoral2
Copy link
Contributor

@fmoral2 fmoral2 commented May 30, 2023

Proposed Changes

  • Creation of a framework to stabilish common standards and best practices on automation so we can be able to share code, implementations, make things more scalable and reliable.

Types of Changes

  • Change in the whole structure and archicteture

Verification

  • Tests ran
  • Add and customize implementations
  • Creation of new tests

Testing

  • Ran each test with all configurations 4 times- OK
  • Code lint: OK
  • Make file | Docker Run | Jenkins updated:  ok

Linked Issues

User-Facing Change


Further Comments

Next Steps:

  • Create a bash script to wrap in a friendly way the run commands for docker and local
  • SUC upgrade improvement
  • Add capability to send all args splited by comma
  • Change the control flow for args by adding a capability to not need anymore to rely on RunOnNode or RunOnHost functions instead just send any args and let the framework handle how to run.
  • Customizable reports
  • Improve logs
  • Totally separation of the entrypoint to the orchestration layer letting each one more atomic
  • Add a relational database with json support

@fmoral2 fmoral2 requested a review from a team as a code owner May 30, 2023 14:39
@fmoral2 fmoral2 force-pushed the master branch 3 times, most recently from 2bfcf42 to a8508a2 Compare May 30, 2023 14:48
@fmoral2 fmoral2 changed the title feat: add acceptance framework feat: add acceptance test framework May 30, 2023
@fmoral2 fmoral2 force-pushed the master branch 3 times, most recently from 907c6ba to 99ea995 Compare May 30, 2023 23:53
@fmoral2 fmoral2 force-pushed the master branch 3 times, most recently from e28ad5d to 0d8005b Compare May 31, 2023 20:02
@fmoral2 fmoral2 requested a review from briandowns May 31, 2023 20:16
@fmoral2 fmoral2 force-pushed the master branch 4 times, most recently from 4fff641 to 0b2d609 Compare June 5, 2023 19:32
@fmoral2 fmoral2 force-pushed the master branch 2 times, most recently from 4e7bc80 to c2057e4 Compare June 6, 2023 19:51
} else if strings.Contains(pod.Name, "apply") &&
strings.Contains(pod.NameSpace, "system-upgrade") {
g.Expect(pod.Status).Should(SatisfyAny(
ContainSubstring("Error"),
Copy link
Contributor

Choose a reason for hiding this comment

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

What if it is actually an Error? Could we wait longer for job to complete?

Copy link
Contributor Author

@fmoral2 fmoral2 Jul 6, 2023

Choose a reason for hiding this comment

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

this error we are only expecting from this namespace system-upgrade ,only for the apply pods which sometimes can occur and then it continues with a new pod to do the job.
If this pod only return error , we will catch later on the upgrade validations , that will try to assert that the nodes are in a new version but will not be .

If we remove this error , this test become too much flaky for no reason.

}

// Pods returns pods parsed from kubectl get pods.
func Pods(print bool) ([]Pod, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's retain the name ParsePods as per its functionality

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure done

}

// Nodes returns nodes parsed from kubectl get nodes.
func Nodes(print bool) ([]Node, error) {
Copy link
Contributor

@ShylajaDevadiga ShylajaDevadiga Jul 7, 2023

Choose a reason for hiding this comment

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

ParseNodes instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure done

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.

4 participants