Skip to content

Backport bugfix: update ring with new ip address when instance is los… #6869

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: release-1.18
Choose a base branch
from
Open

Backport bugfix: update ring with new ip address when instance is los… #6869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d350ed8
Backport bugfix: update ring with new ip address when instance is los…
CharlieTLe Nov 21, 2024
4297f14
update ubuntu action runners (#6584)
dsabsay Feb 13, 2025
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
16 changes: 8 additions & 8 deletions .github/workflows/test-build-deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ on:

jobs:
lint:
runs-on: ubuntu-20.04
runs-on: ubuntu-24.04
container:
image: quay.io/cortexproject/build-image:master-582c03a76
steps:
Expand Down Expand Up @@ -44,7 +44,7 @@ jobs:
run: make BUILD_IN_CONTAINER=false check-white-noise

test:
runs-on: ubuntu-20.04
runs-on: ubuntu-24.04
container:
image: quay.io/cortexproject/build-image:master-582c03a76
steps:
Expand All @@ -64,7 +64,7 @@ jobs:

security:
name: CodeQL
runs-on: ubuntu-20.04
runs-on: ubuntu-24.04
permissions:
actions: read
contents: read
Expand All @@ -87,7 +87,7 @@ jobs:


build:
runs-on: ubuntu-20.04
runs-on: ubuntu-24.04
container:
image: quay.io/cortexproject/build-image:master-582c03a76
steps:
Expand Down Expand Up @@ -132,7 +132,7 @@ jobs:

integration:
needs: build
runs-on: ubuntu-20.04
runs-on: ubuntu-24.04
strategy:
fail-fast: false
matrix:
Expand Down Expand Up @@ -206,7 +206,7 @@ jobs:

integration-configs-db:
needs: build
runs-on: ubuntu-20.04
runs-on: ubuntu-24.04
steps:
- name: Checkout Repo
uses: actions/checkout@v2
Expand All @@ -228,7 +228,7 @@ jobs:
deploy_website:
needs: [build, test]
if: (github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/tags/')) && github.repository == 'cortexproject/cortex'
runs-on: ubuntu-20.04
runs-on: ubuntu-24.04
container:
image: quay.io/cortexproject/build-image:master-582c03a76
steps:
Expand Down Expand Up @@ -270,7 +270,7 @@ jobs:
deploy:
needs: [build, test, lint, integration, integration-configs-db]
if: (github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/tags/')) && github.repository == 'cortexproject/cortex'
runs-on: ubuntu-20.04
runs-on: ubuntu-24.04
container:
image: quay.io/cortexproject/build-image:master-582c03a76
steps:
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## master / unreleased

## 1.18.2 2025-07-09

* [BUGFIX] Backporting Ring: update ring with new ip address when instance is lost, rejoins, but heartbeat is disabled #6271

## 1.18.1 2024-10-14

* [BUGFIX] Backporting upgrade to go 1.22.7 to patch CVE-2024-34155, CVE-2024-34156, CVE-2024-34158 #6217 #6264
Expand Down
2 changes: 1 addition & 1 deletion VERSION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.18.1
1.18.2
3 changes: 3 additions & 0 deletions pkg/ring/lifecycler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,9 @@ func (i *Lifecycler) initRing(ctx context.Context) error {

level.Info(i.logger).Log("msg", "existing entry found in ring", "state", i.GetState(), "tokens", len(tokens), "ring", i.RingName)

// Update the address if it has changed
instanceDesc.Addr = i.Addr
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we up this with the if condition below?

Copy link
Member

Choose a reason for hiding this comment

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

This is just a backport from what is in master. I would implement that suggestion, if needed, in a master PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah i see. agree, we can just do the improvement later on master


// Update the ring if the instance has been changed and the heartbeat is disabled.
// We dont need to update KV here when heartbeat is enabled as this info will eventually be update on KV
// on the next heartbeat
Expand Down
41 changes: 34 additions & 7 deletions pkg/ring/lifecycler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ func testLifecyclerConfig(ringConfig Config, id string) LifecyclerConfig {
return lifecyclerConfig
}

// testLifecyclerConfigWithAddr creates a LifecyclerConfig with the given address.
// This is useful for testing when we want to set the address to a specific value.
func testLifecyclerConfigWithAddr(ringConfig Config, id string, addr string) LifecyclerConfig {
l := testLifecyclerConfig(ringConfig, id)
l.Addr = addr
return l
}

func checkNormalised(d interface{}, id string) bool {
desc, ok := d.(*Desc)
return ok &&
Expand Down Expand Up @@ -644,8 +652,8 @@ func TestRestartIngester_DisabledHeartbeat_unregister_on_shutdown_false(t *testi
}

// Starts Ingester and wait it to became active
startIngesterAndWaitActive := func(ingId string) *Lifecycler {
lifecyclerConfig := testLifecyclerConfig(ringConfig, ingId)
startIngesterAndWaitActive := func(ingId string, addr string) *Lifecycler {
lifecyclerConfig := testLifecyclerConfigWithAddr(ringConfig, ingId, addr)
// Disabling heartBeat and unregister_on_shutdown
lifecyclerConfig.UnregisterOnShutdown = false
lifecyclerConfig.HeartbeatPeriod = 0
Expand All @@ -662,10 +670,10 @@ func TestRestartIngester_DisabledHeartbeat_unregister_on_shutdown_false(t *testi
// test if the ingester 2 became active after:
// * Clean Shutdown (LEAVING after shutdown)
// * Crashes while in the PENDING or JOINING state
l1 := startIngesterAndWaitActive("ing1")
l1 := startIngesterAndWaitActive("ing1", "0.0.0.0")
defer services.StopAndAwaitTerminated(context.Background(), l1) //nolint:errcheck

l2 := startIngesterAndWaitActive("ing2")
l2 := startIngesterAndWaitActive("ing2", "0.0.0.0")

ingesters := poll(func(desc *Desc) bool {
return len(desc.Ingesters) == 2 && desc.Ingesters["ing1"].State == ACTIVE && desc.Ingesters["ing2"].State == ACTIVE
Expand All @@ -684,7 +692,7 @@ func TestRestartIngester_DisabledHeartbeat_unregister_on_shutdown_false(t *testi
assert.Equal(t, LEAVING, ingesters["ing2"].State)

// Start Ingester2 again - Should flip back to ACTIVE in the ring
l2 = startIngesterAndWaitActive("ing2")
l2 = startIngesterAndWaitActive("ing2", "0.0.0.0")
require.NoError(t, services.StopAndAwaitTerminated(context.Background(), l2))

// Simulate ingester2 crash on startup and left the ring with JOINING state
Expand All @@ -698,7 +706,7 @@ func TestRestartIngester_DisabledHeartbeat_unregister_on_shutdown_false(t *testi
})
require.NoError(t, err)

l2 = startIngesterAndWaitActive("ing2")
l2 = startIngesterAndWaitActive("ing2", "0.0.0.0")
require.NoError(t, services.StopAndAwaitTerminated(context.Background(), l2))

// Simulate ingester2 crash on startup and left the ring with PENDING state
Expand All @@ -712,7 +720,26 @@ func TestRestartIngester_DisabledHeartbeat_unregister_on_shutdown_false(t *testi
})
require.NoError(t, err)

l2 = startIngesterAndWaitActive("ing2")
l2 = startIngesterAndWaitActive("ing2", "0.0.0.0")
require.NoError(t, services.StopAndAwaitTerminated(context.Background(), l2))

// Simulate ingester2 crashing and left the ring with ACTIVE state, but when it comes up
// it has a different ip address
startIngesterAndWaitActive("ing2", "0.0.0.0")
ingesters = poll(func(desc *Desc) bool {
return desc.Ingesters["ing2"].State == ACTIVE && desc.Ingesters["ing2"].Addr == "0.0.0.0:1"
})
assert.Equal(t, ACTIVE, ingesters["ing2"].State)
assert.Equal(t, "0.0.0.0:1", ingesters["ing2"].Addr)

l2 = startIngesterAndWaitActive("ing2", "1.1.1.1")

// The ring should have the new ip address
ingesters = poll(func(desc *Desc) bool {
return desc.Ingesters["ing2"].State == ACTIVE && desc.Ingesters["ing2"].Addr == "1.1.1.1:1"
})
assert.Equal(t, ACTIVE, ingesters["ing2"].State)
assert.Equal(t, "1.1.1.1:1", ingesters["ing2"].Addr)
require.NoError(t, services.StopAndAwaitTerminated(context.Background(), l2))
}

Expand Down
Loading