-
Notifications
You must be signed in to change notification settings - Fork 75
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
Support kubernetes service deployer #130
Support kubernetes service deployer #130
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Trends 🧪 |
@ycombinator is ready for review, but please take a look at the elastic/elastic-package#246 to verify if the proposal here is correct. |
@@ -17,8 +17,8 @@ func loadItemContent(itemPath, mediaType string) ([]byte, error) { | |||
return nil, errors.Wrap(err, "reading item file failed") | |||
} | |||
|
|||
if len(itemData) == 0 { | |||
return nil, errors.New("file is empty") | |||
if len(itemData) == 0 && mediaType != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It took me a second to realize that this additional check was required to properly handle the .empty
file case. In the case of the .empty
file, len(itemData) == 0
but mediaType == ""
so this check will evaluate to false
, which is what we want. Without the additional mediaType != ""
clause, this check would have evaluated to true
and returned an error, which is not what we want.
Would you mind adding a comment above this check explaining why we need to check both the length of itemData
and the media type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll merge it once the CI is green and doesn't complain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a minor comment about adding an explanatory comment. 🙂
Otherwise, LGTM.
* Refactor: type visibility, comments * Adjust structures * Use ucfg to support flat and extended manifest files * Fix: make check * Fix: make gomod * Bring back VarValue * Fix: use ucfg in system test runner * Fix: use config instead of yaml * Try: implement Unpacker not ConfigUnpacker
What does this PR do?
This PR defines files used by the Kubernetes service deployer.
Why is it important?
These files will define a custom service deployed on top of the Kubernetes cluster. The Elastic Agent can monitor this service (e.g. nginx deployed in the cluster). If there aren't any files required to be installed in the cluster, it's mandatory to set the
.empty
file as a mark for the test runner to use Kubernetes service deployer (empty directory_dev/deploy/k8s
won't be versioned).Checklist
code/go/internal/validator/test/packages
that prove my change is effective.versions/N/changelog.yml
.Related issues