Add Discovery::groups_alphabetical following kubectl sort order (#887)

* Make Discovery::groups have deterministic order Fixes #886 Signed-off-by: clux <sszynrae@gmail.com> * sort Discovery::groups in kubectl order Signed-off-by: clux <sszynrae@gmail.com> * revert back to hashmap (no need for btree anymore) Signed-off-by: clux <sszynrae@gmail.com> * use peek rather than creating group values twice Signed-off-by: clux <sszynrae@gmail.com> * document Signed-off-by: clux <sszynrae@gmail.com> * update with suggestions Signed-off-by: clux <sszynrae@gmail.com> * update test Signed-off-by: clux <sszynrae@gmail.com> * remove ordering fn since it's simple Signed-off-by: clux <sszynrae@gmail.com> * can avoid making name public since we have a getter Signed-off-by: clux <sszynrae@gmail.com> * ..but then need to update the test Signed-off-by: clux <sszynrae@gmail.com>
kube-rs · May 23, 2022 · 56b173f · 56b173f
1 parent e5300c0
commit 56b173f
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 3 deletions.
diff --git a/kube-client/src/discovery/mod.rs b/kube-client/src/discovery/mod.rs
@@ -21,7 +21,6 @@ enum DiscoveryMode {
 }
 
 impl DiscoveryMode {
-    #[allow(clippy::ptr_arg)] // hashmap complains on &str here
     fn is_queryable(&self, group: &String) -> bool {
         match &self {
             Self::Allow(allowed) => allowed.contains(group),
@@ -138,6 +137,16 @@ impl Discovery {
         self.groups.values()
     }
 
+    /// Returns a sorted vector of all served groups
+    ///
+    /// This vector is in kubectl's normal alphabetical group order
+    pub fn groups_alphabetical(&self) -> Vec<&ApiGroup> {
+        let mut values: Vec<_> = self.groups().collect();
+        // collect to maintain kubectl order of groups
+        values.sort_by_key(|g| g.name());
+        values
+    }
+
     /// Returns the [`ApiGroup`] for a given group if served
     pub fn get(&self, group: &str) -> Option<&ApiGroup> {
         self.groups.get(group)

diff --git a/kube/src/lib.rs b/kube/src/lib.rs
@@ -359,7 +359,7 @@ mod test {
     async fn derived_resources_discoverable() -> Result<(), Box<dyn std::error::Error>> {
         use crate::{
             core::{DynamicObject, GroupVersion, GroupVersionKind},
-            discovery::{self, verbs, Discovery, Scope},
+            discovery::{self, verbs, ApiGroup, Discovery, Scope},
             runtime::wait::{await_condition, conditions},
         };
 
@@ -405,7 +405,10 @@ mod test {
         assert_eq!(ar.kind, gvk.kind);
 
         // check all non-excluded groups that are iterable
-        for group in discovery.groups() {
+        let mut groups = discovery.groups_alphabetical().into_iter();
+        let firstgroup = groups.next().unwrap();
+        assert_eq!(firstgroup.name(), ApiGroup::CORE_GROUP);
+        for group in groups {
             for (ar, caps) in group.recommended_resources() {
                 if !caps.supports_operation(verbs::LIST) {
                     continue;