-
Notifications
You must be signed in to change notification settings - Fork 2
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
base: master
Are you sure you want to change the base?
Conversation
tusharmndr
commented
Nov 25, 2024
•
edited
Loading
edited
- Migrate all Data access to data manager
- Support to read multi drove cluster under a namespace
- Reload all cluster config
src/datamanager.go
Outdated
|
||
for _, data := range dm.namespaces { | ||
for appId, appData := range data.Apps { | ||
allApps[appId] = appData |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
src/datamanager.go
Outdated
"namespace": namespace, | ||
"apps": dm.namespaces[namespace].Apps, | ||
"time": dm.namespaces[namespace].Timestamp, | ||
}).Info("UpdateApps successfully") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
finished successfully
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
src/datamanager.go
Outdated
logger.WithFields(logrus.Fields{ | ||
"operation": operation, | ||
"LastKnownVhosts": dm.LastKnownVhosts, | ||
}).Info("UpdateLastKnownVhosts successfully") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix log messages everywhere
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
finish this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
src/nginx-header.tmpl
Outdated
upstream {{.LeaderVHost}} { | ||
server {{.Leader.Host}}:{{.Leader.Port}}; | ||
|
||
{{if and .Namespaces.stage1.LeaderVHost .Namespaces.stage1.Leader.Endpoint}} |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this 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]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
}() | ||
} | ||
}() | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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"` |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
There was a problem hiding this comment.
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 " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
4bdd515
to
8e43c8f
Compare
nginx-plus: merge fix with master for changes related to new upstreams
fix earlier merge error
added fix for ngnix plus