-
Notifications
You must be signed in to change notification settings - Fork 6
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: add resourcetopo package #43
Conversation
0372f37
to
602327f
Compare
f8aaeb8
to
f583250
Compare
resourcetopo/manager.go
Outdated
return m.storages[key] | ||
} | ||
|
||
func (m *manager) createVirtualStorage(object metav1.TypeMeta) (*nodeStorage, 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.
Change object to meta, or keep them consistent?
return s, nil | ||
} | ||
|
||
informer := getInformer(typeMeta) |
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 whether getInfomer
is nil?
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.
add nil check in manager call func
resourcetopo/manager.go
Outdated
return s, nil | ||
} | ||
|
||
func (m *manager) AddRelationHandler(preOrder, postOrder metav1.TypeMeta, handler RelationHandler) 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.
We can add some comments for those key methods.
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 in types interface define
resourcetopo/manager.go
Outdated
return m, nil | ||
} | ||
|
||
func (m *manager) AddConfig(cfg Config) 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.
addConfig may be more safety.
resourcetopo/node_storage.go
Outdated
s.preOrderResources = make(map[string]*nodeStorage) | ||
} | ||
key := generateMetaKey(preOrder) | ||
preStorage := s.manager.storages[key] |
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.
s.manager.getStorage
bfb4da9
to
33907d2
Compare
resourcetopo/manager.go
Outdated
} | ||
|
||
func (m *manager) AddTopologyConfig(cfg TopologyConfig) error { | ||
if cfg.GetInformer == nil { |
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.
Checking GetInformer
in getOrCreateStorage
is better?
resourcetopo/manager.go
Outdated
return nil | ||
} | ||
|
||
func (m *manager) GetNode(meta *metav1.TypeMeta, name *types.NamespacedName) (NodeInfo, 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.
*metav1.TypeMeta
->
metav1.TypeMeta
Signatures can be the same.
resourcetopo/manager.go
Outdated
relationEventQueue chan relationEvent | ||
nodeEventQueue chan nodeEvent | ||
configLock sync.Mutex | ||
startedCh chan struct{} |
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.
Not really used? Or use started bool
instead?
resourcetopo/node_info.go
Outdated
storageRef: s, | ||
name: name, | ||
namespace: namespace, | ||
// for virtual resource, its lifecycle handled by resourcetopo |
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.
For virtual resource whose lifecycle is handled by resourcetopo
|
||
var _ TopoNodeStorage = &nodeStorage{} | ||
|
||
type nodeStorage struct { |
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.
It is possible to add some annotations to key data.
func (s *nodeStorage) OnAdd(obj interface{}) { | ||
topoObject, ok := obj.(Object) | ||
if !ok { | ||
klog.Errorf("Failed to transform to k8s object %v, ignore this update nodeEvent", obj) |
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.
Case can be used in a consistent.
resourcetopo/node_storage.go
Outdated
} | ||
} | ||
|
||
func (s *nodeStorage) GetNode(namespacedName *types.NamespacedName) (NodeInfo, 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.
When use namespace and name, and when use NamespacedName?
) | ||
|
||
var ( | ||
ReplicaSetMeta = metav1.TypeMeta{Kind: "ReplicaSet", APIVersion: "apps/v1"} |
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 full name or short name in the same? Full name is easier to read :)
cancel() | ||
}) | ||
|
||
It("single pod add", func() { |
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.
Single pod added
or Add single pod
?
PreMeta: StsMeta, | ||
PostMetas: []metav1.TypeMeta{PodMeta}, | ||
Resolve: func(preOrder Object) []ResourceRelation { | ||
stsObj := preOrder.(*appsv1.StatefulSet) |
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.
It is safer to check ok
.
1. support kubernetes or kubernetes likely resources topology management; 2. support add node event handler for node info changed; 3. support add relation event handler for relation existence changed;
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
2. What is the scope of this PR (e.g. component or file name):
resourcetopo/
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
6. Release note
Please refer to Release Notes Language Style Guide to write a quality release note.