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

add forceTcp and keepSocket options #324

Merged
merged 3 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions api/v1/ytsaurus_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,11 @@ type CommonSpec struct {
//+kubebuilder:default:=false
//+optional
UseIPv4 bool `json:"useIpv4"`
//+optional
KeepSocket *bool `json:"keepSocket,omitempty"`
//+optional
ForceTCP *bool `json:"forceTcp,omitempty"`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

//+kubebuilder:default:=true
//+optional
UseShortNames bool `json:"useShortNames"`
Expand Down
10 changes: 10 additions & 0 deletions api/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions config/crd/bases/cluster.ytsaurus.tech_remoteexecnodes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,8 @@ spec:
additionalProperties:
type: string
type: object
forceTcp:
type: boolean
hostNetwork:
default: false
description: Use the host's network namespace for all components.
Expand Down Expand Up @@ -901,6 +903,8 @@ spec:
resources required.
type: object
type: object
keepSocket:
type: boolean
locations:
items:
properties:
Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/cluster.ytsaurus.tech_ytsaurus.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10024,6 +10024,8 @@ spec:
additionalProperties:
type: string
type: object
forceTcp:
type: boolean
hostNetwork:
default: false
description: Use the host's network namespace for all components.
Expand Down Expand Up @@ -12495,6 +12497,8 @@ spec:
jobImage:
description: Default docker image for user jobs.
type: string
keepSocket:
type: boolean
masterCaches:
properties:
affinity:
Expand Down
11 changes: 7 additions & 4 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ _Appears in:_
| `ephemeralCluster` _boolean_ | Allow prioritizing performance over data safety. Useful for tests and experiments. | false | |
| `useIpv6` _boolean_ | | false | |
| `useIpv4` _boolean_ | | false | |
| `keepSocket` _boolean_ | | | |
| `forceTcp` _boolean_ | | | |
| `useShortNames` _boolean_ | | true | |
| `hostNetwork` _boolean_ | Use the host's network namespace for all components. | false | |
| `usePorto` _boolean_ | | false | |
Expand Down Expand Up @@ -1145,6 +1147,8 @@ _Appears in:_
| `ephemeralCluster` _boolean_ | Allow prioritizing performance over data safety. Useful for tests and experiments. | false | |
| `useIpv6` _boolean_ | | false | |
| `useIpv4` _boolean_ | | false | |
| `keepSocket` _boolean_ | | | |
| `forceTcp` _boolean_ | | | |
| `useShortNames` _boolean_ | | true | |
| `hostNetwork` _boolean_ | Use the host's network namespace for all components. | false | |
| `usePorto` _boolean_ | | false | |
Expand Down Expand Up @@ -1662,8 +1666,7 @@ Ytsaurus is the Schema for the ytsaurus API



_Appears in:_
- [YtsaurusValidator](#ytsaurusvalidator)


| Field | Description | Default | Validation |
| --- | --- | --- | --- |
Expand Down Expand Up @@ -1693,6 +1696,8 @@ _Appears in:_
| `ephemeralCluster` _boolean_ | Allow prioritizing performance over data safety. Useful for tests and experiments. | false | |
| `useIpv6` _boolean_ | | false | |
| `useIpv4` _boolean_ | | false | |
| `keepSocket` _boolean_ | | | |
| `forceTcp` _boolean_ | | | |
| `useShortNames` _boolean_ | | true | |
| `hostNetwork` _boolean_ | Use the host's network namespace for all components. | false | |
| `usePorto` _boolean_ | | false | |
Expand Down Expand Up @@ -1729,5 +1734,3 @@ _Appears in:_





Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
chyt={
"address_resolver"={
"enable_ipv4"=%true;
"enable_ipv6"=%true;
"enable_ipv6"=%false;
Copy link
Contributor Author

@leo-astorsky leo-astorsky Aug 14, 2024

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

So it was hardcoded for strawberry, but now it is configured according to the common value, ok

Copy link
Collaborator

@l0kix2 l0kix2 Aug 21, 2024

Choose a reason for hiding this comment

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

On a second thought that may be backward incompatible change and maybe we need to think here more.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So users, who have ipv4=false;ipv6=true or ipv4=true;ipv6=false (assume false/false is invalid configuration) will redeploy strawberry on operator update with this settings, not true/true as before which can break things for them. Not sure yet how problematic such update could be.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose we can describe breaking changes in the next release about strawberry ipv4/6.

retries=1000;
};
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
{
"address_resolver"={
"enable_ipv4"=%true;
"enable_ipv6"=%false;
"keep_socket"=%true;
"force_tcp"=%true;
retries=1000;
};
"solomon_exporter"={
host="{POD_SHORT_HOSTNAME}";
"instance_tags"={
pod="{K8S_POD_NAME}";
};
};
logging={
writers={
debug={
type=file;
"file_name"="/var/log/master.debug.log.zstd";
format="plain_text";
"compression_method"=zstd;
"enable_compression"=%true;
"enable_system_messages"=%true;
"rotation_policy"={
"rotation_period"=900000;
"max_total_size_to_keep"=10737418240;
};
};
error={
type=file;
"file_name"="/var/log/master.error.log";
format="plain_text";
"enable_system_messages"=%true;
};
info={
type=file;
"file_name"="/var/log/master.info.log";
format="plain_text";
"enable_system_messages"=%true;
};
};
rules=[
{
"min_level"=info;
writers=[
info;
];
family="plain_text";
};
{
"min_level"=error;
writers=[
error;
];
family="plain_text";
};
{
"exclude_categories"=[
Bus;
];
"min_level"=debug;
writers=[
debug;
];
family="plain_text";
};
];
"flush_period"=3000;
};
"monitoring_port"=10010;
"rpc_port"=9010;
"timestamp_provider"={
addresses=[
"ms-test-0.masters-test.fake.svc.fake.zone:9010";
];
};
"cluster_connection"={
"cluster_name"=test;
"primary_master"={
addresses=[
"ms-test-0.masters-test.fake.svc.fake.zone:9010";
];
peers=[
{
address="ms-test-0.masters-test.fake.svc.fake.zone:9010";
voting=%true;
};
];
"cell_id"="65726e65-ad6b7562-259-79747361";
};
"discovery_connection"={
addresses=[
"ds-test-0.discovery-test.fake.svc.fake.zone:9020";
"ds-test-1.discovery-test.fake.svc.fake.zone:9020";
"ds-test-2.discovery-test.fake.svc.fake.zone:9020";
];
};
"master_cache"={
addresses=[
"msc-test-0.master-caches-test.fake.svc.fake.zone:9018";
"msc-test-1.master-caches-test.fake.svc.fake.zone:9018";
"msc-test-2.master-caches-test.fake.svc.fake.zone:9018";
];
"cell_id"="65726e65-ad6b7562-259-79747361";
"enable_master_cache_discovery"=%false;
};
};
"cypress_annotations"={
"k8s_node_name"="{K8S_NODE_NAME}";
"k8s_pod_name"="{K8S_POD_NAME}";
"k8s_pod_namespace"="{K8S_POD_NAMESPACE}";
"physical_host"="{K8S_NODE_NAME}";
};
snapshots={
path="/yt/master-data/master-snapshots";
};
changelogs={
path="/yt/master-data/master-changelogs";
};
"use_new_hydra"=%true;
"hydra_manager"={
"max_changelog_count_to_keep"=10;
"max_snapshot_count_to_keep"=1543;
};
"cypress_manager"={
"default_table_replication_factor"=1;
"default_file_replication_factor"=1;
"default_journal_replication_factor"=1;
"default_journal_read_quorum"=1;
"default_journal_write_quorum"=1;
};
"primary_master"={
addresses=[
"ms-test-0.masters-test.fake.svc.fake.zone:9010";
];
peers=[
{
address="ms-test-0.masters-test.fake.svc.fake.zone:9010";
voting=%true;
};
];
"cell_id"="65726e65-ad6b7562-259-79747361";
};
"secondary_masters"=[
];
}
17 changes: 14 additions & 3 deletions pkg/ytconfig/chyt.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,18 @@ type ChytInitCluster struct {
Families []string `yson:"families"`
}

func getStrawberryController() StrawberryController {
type ChytConfig struct {
AddressResolver AddressResolver `yson:"address_resolver"`
}

func getStrawberryController(resolver AddressResolver) (StrawberryController, error) {
chytConfig := ChytConfig{
AddressResolver: resolver,
}
chytYsonConfig, err := marshallYsonConfig(chytConfig)
if err != nil {
return StrawberryController{}, err
}
return StrawberryController{
Strawberry: Strawberry{
Root: "//sys/strawberry",
Expand All @@ -36,10 +47,10 @@ func getStrawberryController() StrawberryController {
RobotUsername: consts.StrawberryControllerUserName,
},
Controllers: map[string]yson.RawValue{
"chyt": yson.RawValue("{address_resolver={enable_ipv4=%true;enable_ipv6=%true;retries=1000}}"),
"chyt": chytYsonConfig,
},
HTTPAPIEndpoint: fmt.Sprintf(":%v", consts.StrawberryHTTPAPIPort),
}
}, nil
}

func getChytInitCluster() ChytInitCluster {
Expand Down
8 changes: 5 additions & 3 deletions pkg/ytconfig/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,11 @@ type ClusterConnection struct {
}

type AddressResolver struct {
EnableIPv4 bool `yson:"enable_ipv4"`
EnableIPv6 bool `yson:"enable_ipv6"`
Retries *int `yson:"retries,omitempty"`
EnableIPv4 bool `yson:"enable_ipv4"`
EnableIPv6 bool `yson:"enable_ipv6"`
KeepSocket *bool `yson:"keep_socket,omitempty"`
ForceTCP *bool `yson:"force_tcp,omitempty"`
Retries *int `yson:"retries,omitempty"`

LocalhostNameOverride *string `yson:"localhost_name_override,omitempty"`
}
Expand Down
11 changes: 9 additions & 2 deletions pkg/ytconfig/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,11 @@ func (g *Generator) fillDriver(c *Driver) {

func (g *BaseGenerator) fillAddressResolver(c *AddressResolver) {
var retries = 1000

c.EnableIPv4 = g.commonSpec.UseIPv4
c.EnableIPv6 = g.commonSpec.UseIPv6
c.KeepSocket = g.commonSpec.KeepSocket
c.ForceTCP = g.commonSpec.ForceTCP

if !c.EnableIPv6 && !c.EnableIPv4 {
// In case when nothing is specified, we prefer IPv4 due to compatibility reasons.
c.EnableIPv4 = true
Expand Down Expand Up @@ -310,7 +312,12 @@ func (g *Generator) GetClusterConnection() ([]byte, error) {
}

func (g *Generator) GetStrawberryControllerConfig() ([]byte, error) {
c := getStrawberryController()
var resolver AddressResolver
g.fillAddressResolver(&resolver)
c, err := getStrawberryController(resolver)
if err != nil {
return nil, err
}
proxy := g.GetHTTPProxiesAddress(consts.DefaultHTTPProxyRole)
c.LocationProxies = []string{proxy}
c.HTTPLocationAliases = map[string][]string{
Expand Down
10 changes: 10 additions & 0 deletions pkg/ytconfig/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,16 @@ func TestGetYQLAgentConfig(t *testing.T) {
canonize.Assert(t, cfg)
}

func TestResolverOptionsKeepSocketAndForceTCP(t *testing.T) {
ytsaurus := getYtsaurusWithEverything()
ytsaurus.Spec.CommonSpec.ForceTCP = ptr.To(true)
ytsaurus.Spec.CommonSpec.KeepSocket = ptr.To(true)
g := NewGenerator(ytsaurus, testClusterDomain)
cfg, err := g.GetMasterConfig(&ytsaurus.Spec.PrimaryMasters)
require.NoError(t, err)
canonize.Assert(t, cfg)
}

func TestGetMasterCachesWithFixedHostsConfig(t *testing.T) {
ytsaurus := withFixedMasterCachesHosts(getYtsaurusWithEverything())
g := NewGenerator(ytsaurus, testClusterDomain)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,8 @@ spec:
additionalProperties:
type: string
type: object
forceTcp:
type: boolean
hostNetwork:
default: false
description: Use the host's network namespace for all components.
Expand Down Expand Up @@ -912,6 +914,8 @@ spec:
resources required.
type: object
type: object
keepSocket:
type: boolean
locations:
items:
properties:
Expand Down
4 changes: 4 additions & 0 deletions ytop-chart/templates/crds/ytsaurus.cluster.ytsaurus.tech.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10035,6 +10035,8 @@ spec:
additionalProperties:
type: string
type: object
forceTcp:
type: boolean
hostNetwork:
default: false
description: Use the host's network namespace for all components.
Expand Down Expand Up @@ -12506,6 +12508,8 @@ spec:
jobImage:
description: Default docker image for user jobs.
type: string
keepSocket:
type: boolean
masterCaches:
properties:
affinity:
Expand Down
Loading