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

dependency update #764

Merged
merged 5 commits into from
Nov 24, 2020
Merged

dependency update #764

merged 5 commits into from
Nov 24, 2020

Conversation

pohly
Copy link
Contributor

@pohly pohly commented Oct 1, 2020

This is the result of go get -u ./... with some manual tweaks:

  • gopkg.in/square/go-jose.v2@v2.2.2 and google.golang.org/grpc@v1.27.0
    because that is what Kubernetes uses and newer versions
    changed the API such that Kubernetes code doesn't compile
  • github.com/operator-framework/operator-sdk/version was made
    an internal package, so we can't use it anymore
  • github.com/operator-framework/operator-sdk/pkg/restmapper was
    removed - we don't seem to need it anymore

Fixes: #756

@pohly pohly mentioned this pull request Oct 2, 2020
@pohly
Copy link
Contributor Author

pohly commented Oct 2, 2020

For some reason, this PR causes the deployment to not match the test:

https://cloudnative-k8sci.southcentralus.cloudapp.azure.com/blue/rest/organizations/jenkins/pipelines/pmem-csi/branches/PR-764/runs/1/nodes/90/steps/142/log/?start=0

[2020-10-01T17:48:57.487Z] �[1mSTEP�[0m: preparing for test "operator API deployment with defaults" in namespace default
[2020-10-01T17:48:58.259Z] Oct  1 17:48:58.012: INFO: No external IP address on nodes, falling back to internal IPs
[2020-10-01T17:48:58.259Z] Oct  1 17:48:58.018: INFO: reusing existing lvm-testing PMEM-CSI components
[2020-10-01T17:48:58.259Z] Oct  1 17:48:58.018: INFO: Waiting for PMEM-CSI driver.

It shouldn't be "reusing existing lvm-testing" here.

@avalluri
Copy link
Contributor

avalluri commented Oct 2, 2020

For some reason, this PR causes the deployment to not match the test:

https://cloudnative-k8sci.southcentralus.cloudapp.azure.com/blue/rest/organizations/jenkins/pipelines/pmem-csi/branches/PR-764/runs/1/nodes/90/steps/142/log/?start=0

[2020-10-01T17:48:57.487Z] �[1mSTEP�[0m: preparing for test "operator API deployment with defaults" in namespace default
[2020-10-01T17:48:58.259Z] Oct  1 17:48:58.012: INFO: No external IP address on nodes, falling back to internal IPs
[2020-10-01T17:48:58.259Z] Oct  1 17:48:58.018: INFO: reusing existing lvm-testing PMEM-CSI components
[2020-10-01T17:48:58.259Z] Oct  1 17:48:58.018: INFO: Waiting for PMEM-CSI driver.

It shouldn't be "reusing existing lvm-testing" here.

It's bit strange, it shouldn't reuse "lvm-testing" for running operator API tests.

@pohly pohly force-pushed the dependencies branch 2 times, most recently from 3654259 to b4ca3f8 Compare October 2, 2020 08:42
@pohly
Copy link
Contributor Author

pohly commented Oct 2, 2020

It's bit strange, it shouldn't reuse "lvm-testing" for running operator API tests.

ginkgo.Describe changed it's behavior from "invoke callback function immediately" to "store function and call later". I suspected that already yesterday, but then didn't see that in a debugger - must have been looking at the wrong code. When I ran it again today, I saw the different stack trace that confirmed this theory. Gets addressed in in bae6ec9

@pohly
Copy link
Contributor Author

pohly commented Nov 11, 2020

We need kubernetes-csi/csi-lib-utils#67 and must suppress process_start_time, otherwise metric gathering fails with:

gathered metric family process_start_time_seconds has help "[ALPHA] Start time of the process since unix epoch in seconds." but should have "Start time of the process since unix epoch in seconds."

@pohly pohly changed the title dependency update WIP: dependency update Nov 11, 2020
The anonymous function is bound to the loop variables which change
their content. This works with the current Ginkgo v1.12.1
where Describe calls the function immediately, but in Ginkgo v1.14.1
that changes and the function is stored and called much later.

To prepare for that, each function instance must be bound to copies of
the loop variables. Otherwise for example a test for
"operator" runs with the deployment for "direct-testing".
This is the result of `go get -u ./...` with some manual tweaks:
- gopkg.in/square/go-jose.v2@v2.2.2 and google.golang.org/grpc@v1.27.0
  because that is what Kubernetes uses and newer versions
  changed the API such that Kubernetes code doesn't compile
- github.com/operator-framework/operator-sdk/version was made
  an internal package, so we can't use it anymore
- github.com/operator-framework/operator-sdk/pkg/restmapper was
  removed - we don't seem to need it anymore
When we get an unexpected status code like 500, the body contains an
explanation of what went wrong.
To see log output, we must initialize the arguments and run the test
with -v=5.
The new release adds support for process_start_time, which we don't
need and don't want because we already get it through the Prometheus
Golang collector.

Worse, if we keep it enabled in the metrics manager, then metrics
collection fails because the two definitions are slightly different,
which Prometheus treats as an error.
@pohly pohly changed the title WIP: dependency update dependency update Nov 23, 2020
@pohly
Copy link
Contributor Author

pohly commented Nov 24, 2020

@avalluri : this is ready for review.

Copy link
Contributor

@avalluri avalluri left a comment

Choose a reason for hiding this comment

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

Looks good.

@avalluri avalluri merged commit fd40020 into intel:devel Nov 24, 2020
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.

add process_start_time_seconds metric
2 participants