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

feat(upgrade_to_v3): add migration for moving configuration keys from Fluentd to sumologic #398

Merged
merged 3 commits into from
Dec 15, 2022

Conversation

kkujawa-sumo
Copy link
Contributor

@kkujawa-sumo kkujawa-sumo commented Nov 29, 2022

Migration for change introduced in SumoLogic/sumologic-kubernetes-collection#2635

@kkujawa-sumo kkujawa-sumo force-pushed the kk-migrate-fluentd-logs-settings branch 2 times, most recently from 1812dbf to b72b0fc Compare November 29, 2022 16:04
@sumo-drosiek sumo-drosiek changed the title feat(upgrade_to_v3): add migration for moving configuraion keys from Fluentd to sumologic feat(upgrade_to_v3): add migration for moving configuration keys from Fluentd to sumologic Nov 30, 2022
@kkujawa-sumo kkujawa-sumo force-pushed the kk-migrate-fluentd-logs-settings branch 5 times, most recently from c2345cf to 1c30fa7 Compare December 15, 2022 09:44
@kkujawa-sumo kkujawa-sumo force-pushed the kk-migrate-fluentd-logs-settings branch 2 times, most recently from 7c2d88f to 4b3f848 Compare December 15, 2022 10:05
@kkujawa-sumo kkujawa-sumo marked this pull request as ready for review December 15, 2022 10:19
@kkujawa-sumo kkujawa-sumo requested a review from a team as a code owner December 15, 2022 10:19
@kkujawa-sumo kkujawa-sumo force-pushed the kk-migrate-fluentd-logs-settings branch from 4b3f848 to 3e25b18 Compare December 15, 2022 10:28
Copy link

@swiatekm swiatekm left a comment

Choose a reason for hiding this comment

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

The logic looks good to me, just had some nitpicks about naming.

@kkujawa-sumo kkujawa-sumo force-pushed the kk-migrate-fluentd-logs-settings branch from 3e25b18 to b6f1861 Compare December 15, 2022 12:50
@kkujawa-sumo
Copy link
Contributor Author

@swiatekm-sumo please take another look

@swiatekm swiatekm self-requested a review December 15, 2022 13:53
Copy link

@swiatekm swiatekm left a comment

Choose a reason for hiding this comment

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

LGTM, 🙈 about the v2 and v3 names

@kkujawa-sumo
Copy link
Contributor Author

LGTM, 🙈 about the v2 and v3 names

Personally I mixed feelings about removing v2 an v3 for example here:

func createSumologicLogs(fluentdLogsV2 *FluentdLogs, sumologicLogsV2 *SumologicLogsV2) *SumologicLogsV3 {
	sumologicLogsV3 := &SumologicLogsV3{}

	if fluentdLogsV2 != nil {
		sumologicLogsV3.Container = createSumologicLogsContainer(fluentdLogsV2.Containers, sumologicLogsV2.Container)
		sumologicLogsV3.Systemd = createSumologicLogsConfig(fluentdLogsV2.Systemd, sumologicLogsV2.Systemd)
		sumologicLogsV3.Kubelet = createSumologicLogsConfig(fluentdLogsV2.Kubelet, nil) // set nil because in v2 there was not any configuration under sumologic.logs.kubelet
		sumologicLogsV3.Default = createSumologicLogsConfig(fluentdLogsV2.Default, nil) // set nil because in v2 there was not any configuration under sumologic.logs.defaultFluentd
	}

	if sumologicLogsV2 != nil {
		sumologicLogsV3.Rest = sumologicLogsV2.Rest
	}
	return sumologicLogsV3
}

this function take sumologicLogsV2 and using this information creates sumologicLogsV3

@kkujawa-sumo kkujawa-sumo force-pushed the kk-migrate-fluentd-logs-settings branch from b6f1861 to 726d55d Compare December 15, 2022 14:47
@kkujawa-sumo kkujawa-sumo force-pushed the kk-migrate-fluentd-logs-settings branch from 726d55d to 016a91d Compare December 15, 2022 14:49
@kkujawa-sumo
Copy link
Contributor Author

kkujawa-sumo commented Dec 15, 2022

I changed to the "new style" and now all V2 in names are changed to Input and all V3 are changed to Output 🎉

@kkujawa-sumo kkujawa-sumo merged commit 9c78e9b into main Dec 15, 2022
@kkujawa-sumo kkujawa-sumo deleted the kk-migrate-fluentd-logs-settings branch December 15, 2022 15:20
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.

None yet

2 participants