-
Notifications
You must be signed in to change notification settings - Fork 141
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
LOG-5030: functional tests for testing log forwarding to Azure Monitor Log #2334
Conversation
@vparfonov: This pull request references LOG-4606 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target either version "4.8." or "openshift-4.8.", but it targets "Logging 5.9.0" instead. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Skipping CI for Draft Pull Request. |
/assign @jcantrill |
@vparfonov: This pull request references LOG-4606 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target either version "4.8." or "openshift-4.8.", but it targets "Logging 5.9.0" instead. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
ee5a3a3
to
70f5f42
Compare
@vparfonov: This pull request references LOG-5030 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.8.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
/hold |
/test functional |
1 similar comment
/test functional |
/hold cancel |
BeforeEach(func() { | ||
framework = functional.NewCollectorFunctionalFrameworkUsingCollector(logging.LogCollectionTypeVector) | ||
// Start mockServer server | ||
mockServer = azuremonitor.NewMockServer(framework.Namespace, azuremonitor.Mockoon) |
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.
Why can this test not rely upon the output container being added to the pod of the functional test? It is just a container and there are benefits to all the containers running in the same pod. If we really can not us the functional framework we should move this test to rely upon the e2e framework but I suspect its possible with the functional one
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.
The problem is: i need to know public URL of Route, before deploy collector, that's why i did it in this way.
See my comment here: forward_to_azuremonitor_test.go#L43.
I will try to find way how do it in one pod
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.
We do we need a public URL? I thought there was a place to set the host? Can we not craft the URL from "localhost" and a port?
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.
Another thing we could do is to modify the framework to mount the /etc/host
file into the collector container using either an emptydir or maybe a configmap. This would provide a way to resolve a name without needing a route
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.
Maybe we can do something with /etc/host, at the end URL must have format: https://some-id.host/
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.
merged to one pod
var AzureHttpDataCollectorApi string | ||
|
||
const ( | ||
image = "mockoon/cli:latest" |
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.
lets tag a version of this and push it to quay.io/openshift-logging so we can ensure it will not change from under us
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.
done
/test functional |
/test functional |
1 similar comment
/test functional |
/test e2e-ocp-target-minus-one |
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.
This is much closer to what I would expect. Good work
Expect(strings.Count(collectorLog, failedReason)).To(BeEquivalentTo(0)) | ||
|
||
//read log from mock server container and validate it | ||
mockoonLogs := readLogFromMockoon(framework) |
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.
My statement of "filter" was more in the direction of we control what we return from the "read" function. Within that function we can return only the bits that are of interest to us. The following lines already do some of this logic by separating receiver logs from the logs we are interested.
/test lint |
/retest all |
@vparfonov: The
The following commands are available to trigger optional jobs:
Use In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/retest |
… Monitor Log Signed-off-by: Vitalii Parfonov <vparfono@redhat.com>
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcantrill, vparfonov The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/test functional |
@vparfonov: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Signed-off-by: Vitalii Parfonov vparfono@redhat.com
Description
This pull request addresses adding functional tests for testing log forwarding to Azure Monitor Log.
Changes:
log-type
header and body fields in each record (message
andlog_type
) to ensure that log records are properly formatted, e.g.:https://<CustomerId>.<Host>/api/logs?api-version=2016-04-01
), including the<CustomerID>
and<Host>
components, to accurately replicate the production environment/cc
/assign
/cherry-pick
Links