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

Update Supported Kubernetes to 1.26, 1.27, 1.28 #3450

Merged
merged 11 commits into from
Oct 24, 2023

Conversation

Kalaiselvi84
Copy link
Contributor

@Kalaiselvi84 Kalaiselvi84 commented Oct 23, 2023

What type of PR is this?

Uncomment only one /kind <> line, press enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking

/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind hotfix
/kind release

What this PR does / Why we need it:

Which issue(s) this PR fixes:

Work on #3371

Special notes for your reviewer:

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 55e366cf-fe1e-473b-9583-7bf005157f8b

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/3450/head:pr_3450 && git checkout pr_3450
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.36.0-dev-0aa88bd-amd64

Copy link
Collaborator

@gongmax gongmax left a comment

Choose a reason for hiding this comment

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

Did you perform step "Regenerate Kubernetes resource includes (e.g. ObjectMeta, PodTemplateSpec)"? I didn't see any change for ObjectMeta and PodTemplateSpec.

@@ -81,6 +81,7 @@ func TestAllocatorAllocate(t *testing.T) {

require.NoError(t, a.Run(ctx))
// wait for it to be up and running
// nolint:staticcheck
err := wait.PollImmediate(time.Second, 10*time.Second, func() (done bool, err error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I saw you fixed this lint warning with PollUntilContextTimeout which looks correct. Do you mind fix these too? We can also do this in a separate PR. Up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, will replace all PollImmediate with PollUntilContextTimeout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've replaced PollImmediate with PollUntilContextTimeout.
Is it fine to replace wait.Poll(time.Second, 10*time.Second, func() (bool, error) with wait.PollUntilContextTimeout(ctx, time.Second, 10*time.Second, true, func(ctx context.Context) (bool, error))?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, wait.Poll can be replaced with wait.PollUntilContextTimeout. Again, up to you whether do that in this PR or a following up PR.

@@ -692,6 +692,7 @@ func Retry(backoff wait.Backoff, fn func() error) error {
return false, nil
}
})
// nolint:staticcheck
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's create an issue to track the fix.

@Kalaiselvi84
Copy link
Contributor Author

Kalaiselvi84 commented Oct 24, 2023

Did you perform step "Regenerate Kubernetes resource includes (e.g. ObjectMeta, PodTemplateSpec)"? I didn't see any change for ObjectMeta and PodTemplateSpec.

Yes, I've executed this step, and within this PR, neither _io.k8s.api.core.v1.PodTemplateSpec.yaml nor _io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta.yaml were modified.
These two files were previously altered by the PR related to the Go script conversion. For reference, I set up the 1.27 cluster before this conversion took place.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: e1c15609-451a-4dbb-8b50-241dcf84ad69

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/3450/head:pr_3450 && git checkout pr_3450
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.36.0-dev-2524a0a-amd64

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 5b74f521-a757-4020-b818-305c95d0fef6

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 6eddc291-731e-45e1-8fca-92ca1fe351ba

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 9a4439d7-632e-49a2-8079-3f670ba0b39d

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 0697fc53-cd79-49e2-bb40-224279f746b2

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/3450/head:pr_3450 && git checkout pr_3450
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.36.0-dev-daa231d-amd64

Copy link
Collaborator

@gongmax gongmax left a comment

Choose a reason for hiding this comment

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

There are some deprecated methods can be replaced. But I'm totally fine to fix it in a following PR if you prefer.

@@ -295,6 +295,7 @@ func (s *SDKServer) WaitForConnection(ctx context.Context) error {
}

try := 0
// nolint:staticcheck
return wait.PollImmediateInfiniteWithContext(ctx, 4*time.Second, func(ctx context.Context) (bool, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can also be replaced with PollUntilContextCancel

@@ -292,6 +292,7 @@ func (f *Framework) WaitForGameServerState(t *testing.T, gs *agonesv1.GameServer
// Each Allocated GameServer gets deleted allocDuration after it was Allocated.
// GameServers will continue to be Allocated until a message is passed to the done channel.
func (f *Framework) CycleAllocations(ctx context.Context, t *testing.T, flt *agonesv1.Fleet, period time.Duration, allocDuration time.Duration) {
// nolint:staticcheck
err := wait.PollImmediateUntil(period, func() (bool, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can also be replaced with PollUntilContextCancel

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gongmax, Kalaiselvi84
Once this PR has been reviewed and has the lgtm label, please assign ericfortin for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Kalaiselvi84
Copy link
Contributor Author

There are some deprecated methods can be replaced. But I'm totally fine to fix it in a following PR if you prefer.

Will create a separate PR for this fix😅

@gongmax gongmax merged commit ace74f6 into googleforgames:main Oct 24, 2023
3 checks passed
@Kalaiselvi84 Kalaiselvi84 deleted the version-update-k8s branch March 15, 2024 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants