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

Multi cluster support #2

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

tusharmndr
Copy link

@tusharmndr tusharmndr commented Nov 25, 2024

  1. Migrate all Data access to data manager
  2. Support to read multi drove cluster under a namespace
  3. Reload all cluster config

@tusharmndr tusharmndr marked this pull request as draft November 25, 2024 04:15

for _, data := range dm.namespaces {
for appId, appData := range data.Apps {
allApps[appId] = appData
Copy link
Contributor

Choose a reason for hiding this comment

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

If multiple namespaces contain same app (possible if multiple clusters have same deployment), this will overwrite

Copy link
Author

Choose a reason for hiding this comment

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

Done

"namespace": namespace,
"apps": dm.namespaces[namespace].Apps,
"time": dm.namespaces[namespace].Timestamp,
}).Info("UpdateApps successfully")
Copy link
Contributor

Choose a reason for hiding this comment

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

finished successfully

Copy link
Author

Choose a reason for hiding this comment

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

Done

logger.WithFields(logrus.Fields{
"operation": operation,
"LastKnownVhosts": dm.LastKnownVhosts,
}).Info("UpdateLastKnownVhosts successfully")
Copy link
Contributor

Choose a reason for hiding this comment

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

fix log messages everywhere

Copy link
Author

Choose a reason for hiding this comment

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

Done

src/drove.go Outdated
@@ -642,12 +750,13 @@ func reloadNginx() error {
}

func reload() error {
return reloadAllApps(false)
return reloadAllApps(config.DroveNamespaces[0].Name, false) //TODO: for for all namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

finish this

Copy link
Author

Choose a reason for hiding this comment

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

Done

upstream {{.LeaderVHost}} {
server {{.Leader.Host}}:{{.Leader.Port}};

{{if and .Namespaces.stage1.LeaderVHost .Namespaces.stage1.Leader.Endpoint}}
Copy link
Contributor

Choose a reason for hiding this comment

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

This will lead to very large breaking change. Put in defaults for namespace if possible

Copy link
Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@santanusinha santanusinha left a comment

Choose a reason for hiding this comment

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

Address comments and backwards compatibility

@@ -22,5 +16,10 @@ maxfailsupstream = 0
failtimeoutupstream = "1s"
slowstartupstream = "0s"

# Drove API
[[namespaces]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Check

Copy link
Author

Choose a reason for hiding this comment

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

Added name

src/drove.go Outdated
// a ticker channel to limit reloads to drove, 1s is enough for now.
ticker := time.NewTicker(1 * time.Second)
droveClient := newDroveClient(name)
ticker := time.NewTicker(time.Duration(droveClient.eventRefreshInterval) * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

Use a default value if not provided in config

Copy link
Author

Choose a reason for hiding this comment

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

Done

}()
}
}()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The eventqueue is removed? what if reload takes more than 5 secs?

Copy link
Author

Choose a reason for hiding this comment

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

added

@@ -51,43 +50,44 @@ type Vhosts struct {
Vhosts map[string]bool
}

type DroveNamespace struct {
Name string `json:"-" toml:"name"`
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be mandatory

Copy link
Author

Choose a reason for hiding this comment

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

Done

src/nixy.go Outdated
select {
case eventqueue <- true: // Add reload to our queue channel, unless it is full of course.
}).Info("Reload triggered")
queued := forceReload()
Copy link
Contributor

Choose a reason for hiding this comment

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

The event queue was present to ensure that reload always happens only from one thread.

Copy link
Author

Choose a reason for hiding this comment

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

Done, just renamed

src/nixy.toml Outdated
drove = ["https://localhost:8080"] # add all HA cluster nodes in priority order.
user = "" # leave empty if no auth is required.
pass = ""
access_token = "O-Bearer "
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

Copy link
Author

Choose a reason for hiding this comment

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

Done

src/nixy.toml Outdated
drove = ["https://localhost:8080"] # add all HA cluster nodes in priority order.
user = "" # leave empty if no auth is required.
pass = ""
access_token = "O-Bearer "
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

Copy link
Author

Choose a reason for hiding this comment

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

Done

Namespaces map[string]NamespaceRenderingData `json:"namespaces"`
}

func reload() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we expose a channel / queue here to receive a signal for reload.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@tusharmndr tusharmndr marked this pull request as ready for review December 6, 2024 04:50
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.

2 participants