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

[FlowAggregator] Upgrade to ClickHouse Go client v2 #4901

Closed
antoninbas opened this issue Apr 25, 2023 · 4 comments · Fixed by #5020
Closed

[FlowAggregator] Upgrade to ClickHouse Go client v2 #4901

antoninbas opened this issue Apr 25, 2023 · 4 comments · Fixed by #5020
Assignees
Labels
area/flow-visibility/aggregator Issues or PRs related to Flow Aggregator kind/task Categorizes issue or PR as related to a routine task that needs to be performed

Comments

@antoninbas
Copy link
Contributor

ClickHouse Go client v1 is in maintenance-only mode. All new features are being introduced in v2.
It seems that v2 of the client also received performance improvements: https://github.com/ClickHouse/clickhouse-go#benchmark

There are some breaking changes between v1 and v2, which have been documented here: https://github.com/ClickHouse/clickhouse-go/blob/main/v1_v2_CHANGES.md

We should upgrade the Antrea Flow Aggregator to use v2 of the client, for future-proofness and to benefit from the improvements introduced in v2.

@antoninbas antoninbas added kind/task Categorizes issue or PR as related to a routine task that needs to be performed area/flow-visibility/aggregator Issues or PRs related to Flow Aggregator labels Apr 25, 2023
@heanlan
Copy link
Contributor

heanlan commented May 1, 2023

Hi @antoninbas , I got some dependencies conflicts on go.opentelemetry.io/otel

  • The latest ClickHouse Go client v2 github.com/ClickHouse/clickhouse-go/v2 v2.9.1 requires go.opentelemetry.io/otel v1.13.0, even v2.0.0 requires go.opentelemetry.io/otel v1.3.0
  • Antrea depends on k8s.io/apiserver v0.24.0, which depends on go.opentelemetry.io/otel v0.20.0
  • go.opentelemetry.io/otel introduces incompatible changes between v0.x.y and v1.x.y

There are a lot of related issues in opentelemetry-go repository: e.g. open-telemetry/opentelemetry-go#3292

These are what I got when running go mod tidy after upgrading ClickHouse Go client to v2.9.1. For example, go.opentelemetry.io/otel/metric/registry no longer exist in v1 modules

antrea.io/antrea/cmd/antrea-agent imports
	k8s.io/apiserver/pkg/server/options imports
	go.opentelemetry.io/otel/semconv: module go.opentelemetry.io/otel@latest found (v1.15.0), but does not contain package go.opentelemetry.io/otel/semconv
antrea.io/antrea/cmd/antrea-agent imports
	k8s.io/apiserver/pkg/server/options imports
	go.opentelemetry.io/otel/exporters/otlp/otlpgrpc imports
	go.opentelemetry.io/otel/exporters/otlp/internal/transform imports
	go.opentelemetry.io/otel/metric/number: module go.opentelemetry.io/otel/metric@latest found (v0.38.0), but does not contain package go.opentelemetry.io/otel/metric/number
antrea.io/antrea/cmd/antrea-agent imports
	k8s.io/apiserver/pkg/server/options imports
	go.opentelemetry.io/otel/exporters/otlp/otlpgrpc imports
	go.opentelemetry.io/otel/exporters/otlp imports
	go.opentelemetry.io/otel/sdk/metric/controller/basic imports
	go.opentelemetry.io/otel/metric/registry: module go.opentelemetry.io/otel/metric@latest found (v0.38.0), but does not contain package go.opentelemetry.io/otel/metric/registry
antrea.io/antrea/cmd/antrea-agent imports
	k8s.io/apiserver/pkg/server/options imports
	go.opentelemetry.io/otel/exporters/otlp/otlpgrpc imports
	go.opentelemetry.io/otel/exporters/otlp imports
	go.opentelemetry.io/otel/sdk/metric/controller/basic imports
	go.opentelemetry.io/otel/sdk/metric imports
	go.opentelemetry.io/otel/internal/metric imports
	go.opentelemetry.io/otel/metric/sdkapi: module go.opentelemetry.io/otel/metric@latest found (v0.38.0), but does not contain package go.opentelemetry.io/otel/metric/sdkapi

I see k8s.io/apiserver latest depends on go.opentelemetry.io/otel v1.10.0 : https://github.com/kubernetes/apiserver/blob/2dff60bb4d1c1e9877bb624908af0d53e51634c0/go.mod#L29 . Do we consider upgrade k8s.io/apiserver first? Do you have other suggestions? Thanks

@antoninbas
Copy link
Contributor Author

You can upgrade Antrea to use version v0.27.1 (most recent) of the K8s dependencies. Typically we try to keep these dependencies "current", but the update process can be a bit involved. This seems like a good time to do it. You should do it as a separate PR (i.e., not part of the same PR as the ClickHouse update).

Refer to these 2 PRs for the previous update:

(it should have been a single PR, but I forgot something in the first one)

@heanlan
Copy link
Contributor

heanlan commented May 2, 2023

I noticed K8s v0.27.1 uses Go 1.20. Do we want to 1) open another PR to upgrade Go version or 2) only upgrade K8s lib to v0.26.4 which uses Go 1.19?

Given the scope of this upgrade, Elton suggested me to also cc @tnqn and @luolanzone in case they have any objections

@antoninbas
Copy link
Contributor Author

v0.26.4 is fine by me

heanlan added a commit to heanlan/antrea that referenced this issue May 4, 2023
* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
  sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
  Similar changes happened on sets.Int32, sets.Int64
* k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
  implementation of Destroy() method
* k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
  not doing anything. The call in third_party/ipam/nodeipam was removed
  accordingly. Refer to PR: kubernetes/kubernetes#113054
* upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
  multicluster unit-tests were impacted
* k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
  replaced with StreamWithContext

Related to antrea-io#4901

Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue May 5, 2023
* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
  sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
  Similar changes happened on sets.Int32, sets.Int64
* k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
  implementation of Destroy() method
* k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
  not doing anything. The call in third_party/ipam/nodeipam was removed
  accordingly. Refer to PR: kubernetes/kubernetes#113054
* upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
  timeout was removed from BeforeSuite, and was refactored referencing to
  onsi/ginkgo#882
* k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
  replaced with StreamWithContext

Related to antrea-io#4901

Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue May 16, 2023
* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
  sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
  Similar changes happened on sets.Int32, sets.Int64
* k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
  implementation of Destroy() method
* k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
  not doing anything. The call in third_party/ipam/nodeipam was removed
  accordingly. Refer to PR: kubernetes/kubernetes#113054
* upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
  timeout was removed from BeforeSuite, and was refactored referencing to
  onsi/ginkgo#882
* k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
  replaced with StreamWithContext

Related to antrea-io#4901

Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue May 16, 2023
* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
  sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
  Similar changes happened on sets.Int32, sets.Int64
* k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
  implementation of Destroy() method
* k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
  not doing anything. The call in third_party/ipam/nodeipam was removed
  accordingly. Refer to PR: kubernetes/kubernetes#113054
* upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
  timeout was removed from BeforeSuite, and was refactored referencing to
  onsi/ginkgo#882
* k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
  replaced with StreamWithContext

Related to antrea-io#4901

Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue May 16, 2023
* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
  sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
  Similar changes happened on sets.Int32, sets.Int64
* k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
  implementation of Destroy() method
* k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
  not doing anything. The call in third_party/ipam/nodeipam was removed
  accordingly. Refer to PR: kubernetes/kubernetes#113054
* upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
  timeout was removed from BeforeSuite, and was refactored referencing to
  onsi/ginkgo#882
* k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
  replaced with StreamWithContext

Related to antrea-io#4901

Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue May 16, 2023
* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
  sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
  Similar changes happened on sets.Int32, sets.Int64
* k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
  implementation of Destroy() method
* k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
  not doing anything. The call in third_party/ipam/nodeipam was removed
  accordingly. Refer to PR: kubernetes/kubernetes#113054
* upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
  timeout was removed from BeforeSuite, and was refactored referencing to
  onsi/ginkgo#882
* k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
  replaced with StreamWithContext

Related to antrea-io#4901

Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue May 16, 2023
* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
  sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
  Similar changes happened on sets.Int32, sets.Int64
* k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
  implementation of Destroy() method
* k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
  not doing anything. The call in third_party/ipam/nodeipam was removed
  accordingly. Refer to PR: kubernetes/kubernetes#113054
* upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
  timeout was removed from BeforeSuite, and was refactored referencing to
  onsi/ginkgo#882
* k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
  replaced with StreamWithContext

Related to antrea-io#4901

Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue May 17, 2023
* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
  sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
  Similar changes happened on sets.Int32, sets.Int64
* k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
  implementation of Destroy() method
* k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
  not doing anything. The call in third_party/ipam/nodeipam was removed
  accordingly. Refer to PR: kubernetes/kubernetes#113054
* upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
  timeout was removed from BeforeSuite, and was refactored referencing to
  onsi/ginkgo#882
* k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
  replaced with StreamWithContext

Related to antrea-io#4901

Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue May 18, 2023
* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
  sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
  Similar changes happened on sets.Int32, sets.Int64
* k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
  implementation of Destroy() method
* k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
  not doing anything. The call in third_party/ipam/nodeipam was removed
  accordingly. Refer to PR: kubernetes/kubernetes#113054
* upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
  timeout was removed from BeforeSuite, and was refactored referencing to
  onsi/ginkgo#882
* k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
  replaced with StreamWithContext

Related to antrea-io#4901

Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue May 18, 2023
* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
  sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
  Similar changes happened on sets.Int32, sets.Int64
* k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
  implementation of Destroy() method
* k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
  not doing anything. The call in third_party/ipam/nodeipam was removed
  accordingly. Refer to PR: kubernetes/kubernetes#113054
* upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
  timeout was removed from BeforeSuite, and was refactored referencing to
  onsi/ginkgo#882
* k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
  replaced with StreamWithContext

Related to antrea-io#4901

Signed-off-by: heanlan <hanlan@vmware.com>
tnqn pushed a commit that referenced this issue May 19, 2023
* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
  sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
  Similar changes happened on sets.Int32, sets.Int64
* k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
  implementation of Destroy() method
* k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
  not doing anything. The call in third_party/ipam/nodeipam was removed
  accordingly. Refer to PR: kubernetes/kubernetes#113054
* upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
  timeout was removed from BeforeSuite, and was refactored referencing to
  onsi/ginkgo#882
* k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
  replaced with StreamWithContext
* make codegen and manifests
* allow access from container users to git directories

Related to #4901

Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue May 22, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2, replaced
sql/database interface to clickhouse interface to benefit from v2
and to better assist future supports.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue May 22, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2, replaced
sql/database interface to clickhouse interface to benefit from v2
and to better assist future supports.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue May 23, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2, replaced
sql/database interface to clickhouse interface to benefit from v2
and to better assist future supports.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue Jun 1, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2, replaced
sql/database interface to clickhouse interface to benefit from v2
and to better assist future supports.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue Jun 1, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2, replaced
sql/database interface to clickhouse interface to benefit from v2
and to better assist future supports.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue Jun 1, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2, replaced
sql/database interface to clickhouse interface to benefit from v2
and to better assist future supports.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue Jun 1, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2, replaced
sql/database interface to clickhouse interface to benefit from v2
and to better assist future supports.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue Jun 1, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2, replaced
sql/database interface to clickhouse interface to benefit from v2
and to better assist future supports.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue Jun 1, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2, replaced
sql/database interface to clickhouse interface to benefit from v2
and to better assist future supports.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue Jun 2, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2, replaced
sql/database interface to clickhouse interface to benefit from v2
and to better assist future supports.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue Jun 2, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2 to
better assist future supports. In terms of the method of creating
connection, we replace DSN with OpenDB in database/sql interface.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue Jun 2, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2 to
better assist future supports. In terms of the method of creating
connection, we replace DSN with OpenDB in database/sql interface.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
heanlan added a commit to heanlan/antrea that referenced this issue Jun 2, 2023
We upgrade Flow Aggregator to use ClickHouse go client v2 to
better assist future supports. In terms of the method of creating
connection, we replace DSN with OpenDB in database/sql interface.

Fixes: antrea-io#4901
Signed-off-by: heanlan <hanlan@vmware.com>
ceclinux pushed a commit to ceclinux/antrea that referenced this issue Jun 5, 2023
* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
  sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
  Similar changes happened on sets.Int32, sets.Int64
* k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
  implementation of Destroy() method
* k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
  not doing anything. The call in third_party/ipam/nodeipam was removed
  accordingly. Refer to PR: kubernetes/kubernetes#113054
* upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
  timeout was removed from BeforeSuite, and was refactored referencing to
  onsi/ginkgo#882
* k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
  replaced with StreamWithContext
* make codegen and manifests
* allow access from container users to git directories

Related to antrea-io#4901

Signed-off-by: heanlan <hanlan@vmware.com>
antoninbas pushed a commit that referenced this issue Jun 6, 2023
We upgrade Flow Aggregator to use the ClickHouse go client v2,
for better performance and future-proofness.
We are using the standard database/sql interface, and not the new
"native" ClickHouse interface,
To open the connection, we replace the DSN with the OpenDB function
with options.

Fixes: #4901

Signed-off-by: heanlan <hanlan@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/flow-visibility/aggregator Issues or PRs related to Flow Aggregator kind/task Categorizes issue or PR as related to a routine task that needs to be performed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants