Skip to content
This repository was archived by the owner on Mar 11, 2022. It is now read-only.

Fix unreliable webhook triggers#354

Merged
seanmalloy merged 7 commits intomasterfrom
issue-336
Jul 16, 2020
Merged

Fix unreliable webhook triggers#354
seanmalloy merged 7 commits intomasterfrom
issue-336

Conversation

@Smiley73
Copy link
Copy Markdown
Contributor

@Smiley73 Smiley73 commented Jul 9, 2020

Description

This pull request fixes the issue with unreliable webhook triggers.

Using GenericEvents and a channel produces inconsistent results. Most OSS operators don't implement this functionality. Switching back to creating the gitopsconfig job directly (Which is the same the reconciler would do, except this works consistently).

Fixes #336

This PR also adds 2 new parameters to scripts/e2e-test.sh in order to speed up local testing cycles:

-d|--nodeployment Skip the deployment of the operator
-n|--noimages Skip building the images
-r|--run=<test name> Only run the specified Go e2e test. This will skip all others.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Unit tests and e2e tests updated
  • Documentation updated

Using GenericEvents and a channel produces inconsistent results.
Most OSS operators don't implement this functionality.
Switching back to creating the gitopsconfig job directly.
(Which is the same the rconciler would do, except this works consistently)
@Smiley73 Smiley73 added the wip Work In Progress label Jul 9, 2020
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 9, 2020

Codecov Report

Merging #354 into master will increase coverage by 0.18%.
The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #354      +/-   ##
==========================================
+ Coverage   24.81%   25.00%   +0.18%     
==========================================
  Files           6        6              
  Lines         540      536       -4     
==========================================
  Hits          134      134              
+ Misses        377      373       -4     
  Partials       29       29              
Impacted Files Coverage Δ
pkg/handler/handler.go 8.95% <0.00%> (ø)
pkg/controller/gitopsconfig/controller.go 40.12% <60.00%> (+0.49%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b32d8b...8f8945a. Read the comment docs.

@Smiley73 Smiley73 removed the wip Work In Progress label Jul 10, 2020
Comment thread scripts/e2e-test.sh
echo "Error: Argument for $1 is missing" >&2
exit 1
fi
;;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Curious as to why this is done via environment variables vs managing the settings in the code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's about the scope of the variables. Using export allows us to be more flexible within the shell script. The env variables never make it to the parent shell, so there's no concern about it changing anything important.
We also need to use environment variables so that we can control how make behaves when called from .travis.yml

jimmyfigiel
jimmyfigiel previously approved these changes Jul 14, 2020
@seanmalloy seanmalloy added this to the v0.1.7 milestone Jul 16, 2020
@seanmalloy seanmalloy merged commit faf4cc5 into master Jul 16, 2020
@seanmalloy seanmalloy deleted the issue-336 branch July 16, 2020 04:27
@seanmalloy seanmalloy added the bug Something isn't working label Jul 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Development

Successfully merging this pull request may close these issues.

Webhook only kicking off one of multiple gitops configs for same repo and branch

3 participants